[Webkit-unassigned] [Bug 184012] Unable to use HashMap<WebCore::SecurityOriginData, Ref<WebSWServerToContextConnection>> type in StorageProcess.h

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=184012

--- Comment #4 from Chris Dumez <cdumez at apple.com> ---
(In reply to Chris Dumez from comment #3)
> (In reply to Chris Dumez from comment #2)
> > Using HashMap<RefPtr<WebCore::SecurityOrigin>,
> > Ref<WebSWServerToContextConnection>> works. I believe the different is that
> > emptyValueIsZero for both the Key and the Value.
> > 
> > emptyValueIsZero is false when the key is a SecurityOriginData. So
> > KeyValuePairHashTraits::emptyValueIsZero goes from true to false and it
> > causes failures.
> 
> As a result, it ends up using a different template specialization for
> HashTableBucketInitializer:
>     template<bool emptyValueIsZero> struct HashTableBucketInitializer;
> 
>     template<> struct HashTableBucketInitializer<false> {
>         template<typename Traits, typename Value> static void
> initialize(Value& bucket)
>         {
>             new (NotNull, std::addressof(bucket))
> Value(Traits::emptyValue());
>         }
>     };
> 
>     template<> struct HashTableBucketInitializer<true> {
>         template<typename Traits, typename Value> static void
> initialize(Value& bucket)
>         {
>             // This initializes the bucket without copying the empty value.
>             // That makes it possible to use this with types that don't
> support copying.
>             // The memset to 0 looks like a slow operation but is optimized
> by the compilers.
>             memset(std::addressof(bucket), 0, sizeof(bucket));
>         }
>     };

So instead of doing a memset zero, we now rely on Traits::emptyValue(), which for Ref<T> is static Ref<P> emptyValue() { return HashTableEmptyValue; }

This is implemented as:
Ref(HashTableEmptyValueType) : m_ptr(hashTableEmptyValue()) { }

hashTableDeletedValue() returns nullptr.

So when we construct a KeyValuePair:
    template<typename K, typename V>
    KeyValuePair(K&& key, V&& value)
        : key(std::forward<K>(key))
        , value(std::forward<V>(value))
    {
    }

And V is HashTraits<Ref<T>>::emptyValue(), then it calls the Ref<T>(Ref<t>&&) move constructor, which ends up calling leakRef() on the Ref. This is implemented as:
    T& leakRef() WARN_UNUSED_RETURN
    {
        ASSERT(m_ptr);
        / ...
    }

We hit this assertion since m_ptr is nullptr when the Ref<> is an empty HashTable value :/

Advice on how to best address this would be appreciated:
1. Dropping this assertion would be unfortunate since it is useful to catch bugs
2. Using another value than nullptr as empty HashTable value maybe?

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20180326/053251b9/attachment-0001.html>