Bug 184012 – Unable to use HashMap<WebCore::SecurityOriginData, Ref<WebSWServerToContextConnection>> type in StorageProcess.h

• Component: Web Template Framework
• Status: NEW

[Chris Dumez]

Unable to use HashMap<WebCore::SecurityOriginData, RefPtr<WebSWServerToContextConnection>> type in StorageProcess.h. It causes an assertion to be hit in debug:
Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   com.apple.JavaScriptCore      	0x00000001243b97b4 WTFCrash + 36 (Assertions.cpp:271)
1   com.apple.WebKit              	0x000000010f510de6 WTF::Ref<WebKit::WebSWServerToContextConnection, WTF::DumbPtrTraits<WebKit::WebSWServerToContextConnection> >::leakRef() + 70 (Ref.h:133)
2   com.apple.WebKit              	0x000000010f510d51 WTF::Ref<WebKit::WebSWServerToContextConnection, WTF::DumbPtrTraits<WebKit::WebSWServerToContextConnection> >::Ref(WTF::Ref<WebKit::WebSWServerToContextConnection, WTF::DumbPtrTraits<WebKit::WebSWServerToContextConnection> >&&) + 33 (Ref.h:75)
3   com.apple.WebKit              	0x000000010f510d1d WTF::Ref<WebKit::WebSWServerToContextConnection, WTF::DumbPtrTraits<WebKit::WebSWServerToContextConnection> >::Ref(WTF::Ref<WebKit::WebSWServerToContextConnection, WTF::DumbPtrTraits<WebKit::WebSWServerToContextConnection> >&&) + 29 (Ref.h:78)
4   com.apple.WebKit              	0x000000010f510cec WTF::KeyValuePair<WebCore::SecurityOriginData, WTF::Ref<WebKit::WebSWServerToContextConnection, WTF::DumbPtrTraits<WebKit::WebSWServerToContextConnection> > >::KeyValuePair<WebCore::SecurityOriginData, WTF::Ref<WebKit::WebSWServerToContextConnection, WTF::DumbPtrTraits<WebKit::WebSWServerToContextConnection> > >(WebCore::SecurityOriginData&&, WTF::Ref<WebKit::WebSWServerToContextConnection, WTF::DumbPtrTraits<WebKit::WebSWServerToContextConnection> >&&) + 76 (KeyValuePair.h:46)
5   com.apple.WebKit              	0x000000010f510c25 WTF::KeyValuePair<WebCore::SecurityOriginData, WTF::Ref<WebKit::WebSWServerToContextConnection, WTF::DumbPtrTraits<WebKit::WebSWServerToContextConnection> > >::KeyValuePair<WebCore::SecurityOriginData, WTF::Ref<WebKit::WebSWServerToContextConnection, WTF::DumbPtrTraits<WebKit::WebSWServerToContextConnection> > >(WebCore::SecurityOriginData&&, WTF::Ref<WebKit::WebSWServerToContextConnection, WTF::DumbPtrTraits<WebKit::WebSWServerToContextConnection> >&&) + 37 (KeyValuePair.h:46)
6   com.apple.WebKit              	0x000000010f510bb9 WTF::KeyValuePairHashTraits<WTF::HashTraits<WebCore::SecurityOriginData>, WTF::HashTraits<WTF::Ref<WebKit::WebSWServerToContextConnection, WTF::DumbPtrTraits<WebKit::WebSWServerToContextConnection> > > >::emptyValue() + 57 (HashTraits.h:292)
7   com.apple.WebKit              	0x000000010f510b79 void WTF::HashTableBucketInitializer<false>::initialize<WTF::HashMap<WebCore::SecurityOriginData, WTF::Ref<WebKit::WebSWServerToContextConnection, WTF::DumbPtrTraits<WebKit::WebSWServerToContextConnection> >, WebCore::SecurityOriginDataHash, WTF::HashTraits<WebCore::SecurityOriginData>, WTF::HashTraits<WTF::Ref<WebKit::WebSWServerToContextConnection, WTF::DumbPtrTraits<WebKit::WebSWServerToContextConnection> > > >::KeyValuePairTraits, WTF::KeyValuePair<WebCore::SecurityOriginData, WTF::Ref<WebKit::WebSWServerToContextConnection, WTF::DumbPtrTraits<WebKit::WebSWServerToContextConnection> > > >(WTF::KeyValuePair<WebCore::SecurityOriginData, WTF::Ref<WebKit::WebSWServerToContextConnection, WTF::DumbPtrTraits<WebKit::WebSWServerToContextConnection> > >&) + 57 (HashTable.h:841)
8   com.apple.WebKit              	0x000000010f510b35 WTF::HashTable<WebCore::SecurityOriginData, WTF::KeyValuePair<WebCore::SecurityOriginData, WTF::Ref<WebKit::WebSWServerToContextConnection, WTF::DumbPtrTraits<WebKit::WebSWServerToContextConnection> > >, WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<WebCore::SecurityOriginData, WTF::Ref<WebKit::WebSWServerToContextConnection, WTF::DumbPtrTraits<WebKit::WebSWServerToContextConnection> > > >, WebCore::SecurityOriginDataHash, WTF::HashMap<WebCore::SecurityOriginData, WTF::Ref<WebKit::WebSWServerToContextConnection, WTF::DumbPtrTraits<WebKit::WebSWServerToContextConnection> >, WebCore::SecurityOriginDataHash, WTF::HashTraits<WebCore::SecurityOriginData>, WTF::HashTraits<WTF::Ref<WebKit::WebSWServerToContextConnection, WTF::DumbPtrTraits<WebKit::WebSWServerToContextConnection> > > >::KeyValuePairTraits, WTF::HashTraits<WebCore::SecurityOriginData> >::initializeBucket(WTF::KeyValuePair<WebCore::SecurityOriginData, WTF::Ref<WebKit::WebSWServerToContextConnection, WTF::DumbPtrTraits<WebKit::WebSWServerToContextConnection> > >&) + 21 (HashTable.h:858)
9   com.apple.WebKit              	0x000000010f51095b WTF::HashTable<WebCore::SecurityOriginData, WTF::KeyValuePair<WebCore::SecurityOriginData, WTF::Ref<WebKit::WebSWServerToContextConnection, WTF::DumbPtrTraits<WebKit::WebSWServerToContextConnection> > >, WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<WebCore::SecurityOriginData, WTF::Ref<WebKit::WebSWServerToContextConnection, WTF::DumbPtrTraits<WebKit::WebSWServerToContextConnection> > > >, WebCore::SecurityOriginDataHash, WTF::HashMap<WebCore::SecurityOriginData, WTF::Ref<WebKit::WebSWServerToContextConnection, WTF::DumbPtrTraits<WebKit::WebSWServerToContextConnection> >, WebCore::SecurityOriginDataHash, WTF::HashTraits<WebCore::SecurityOriginData>, WTF::HashTraits<WTF::Ref<WebKit::WebSWServerToContextConnection, WTF::DumbPtrTraits<WebKit::WebSWServerToContextConnection> > > >::KeyValuePairTraits, WTF::HashTraits<WebCore::SecurityOriginData> >::allocateTable(unsigned int) + 75 (HashTable.h:1148)
10  com.apple.WebKit              	0x000000010f5106e8 WTF::HashTable<WebCore::SecurityOriginData, WTF::KeyValuePair<WebCore::SecurityOriginData, WTF::Ref<WebKit::WebSWServerToContextConnection, WTF::DumbPtrTraits<WebKit::WebSWServerToContextConnection> > >, WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<WebCore::SecurityOriginData, WTF::Ref<WebKit::WebSWServerToContextConnection, WTF::DumbPtrTraits<WebKit::WebSWServerToContextConnection> > > >, WebCore::SecurityOriginDataHash, WTF::HashMap<WebCore::SecurityOriginData, WTF::Ref<WebKit::WebSWServerToContextConnection, WTF::DumbPtrTraits<WebKit::WebSWServerToContextConnection> >, WebCore::SecurityOriginDataHash, WTF::HashTraits<WebCore::SecurityOriginData>, WTF::HashTraits<WTF::Ref<WebKit::WebSWServerToContextConnection, WTF::DumbPtrTraits<WebKit::WebSWServerToContextConnection> > > >::KeyValuePairTraits, WTF::HashTraits<WebCore::SecurityOriginData> >::rehash(unsigned int, WTF::KeyValuePair<WebCore::SecurityOriginData, WTF::Ref<WebKit::WebSWServerToContextConnection, WTF::DumbPtrTraits<WebKit::WebSWServerToContextConnection> > >*) + 72 (HashTable.h:1197)
11  com.apple.WebKit              	0x000000010f518385 WTF::HashTable<WebCore::SecurityOriginData, WTF::KeyValuePair<WebCore::SecurityOriginData, WTF::Ref<WebKit::WebSWServerToContextConnection, WTF::DumbPtrTraits<WebKit::WebSWServerToContextConnection> > >, WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<WebCore::SecurityOriginData, WTF::Ref<WebKit::WebSWServerToContextConnection, WTF::DumbPtrTraits<WebKit::WebSWServerToContextConnection> > > >, WebCore::SecurityOriginDataHash, WTF::HashMap<WebCore::SecurityOriginData, WTF::Ref<WebKit::WebSWServerToContextConnection, WTF::DumbPtrTraits<WebKit::WebSWServerToContextConnection> >, WebCore::SecurityOriginDataHash, WTF::HashTraits<WebCore::SecurityOriginData>, WTF::HashTraits<WTF::Ref<WebKit::WebSWServerToContextConnection, WTF::DumbPtrTraits<WebKit::WebSWServerToContextConnection> > > >::KeyValuePairTraits, WTF::HashTraits<WebCore::SecurityOriginData> >::expand(WTF::KeyValuePair<WebCore::SecurityOriginData, WTF::Ref<WebKit::WebSWServerToContextConnection, WTF::DumbPtrTraits<WebKit::WebSWServerToContextConnection> > >*) + 117 (HashTable.h:1174)
12  com.apple.WebKit              	0x000000010f51809c WTF::HashTableAddResult<WTF::HashTableIterator<WebCore::SecurityOriginData, WTF::KeyValuePair<WebCore::SecurityOriginData, WTF::Ref<WebKit::WebSWServerToContextConnection, WTF::DumbPtrTraits<WebKit::WebSWServerToContextConnection> > >, WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<WebCore::SecurityOriginData, WTF::Ref<WebKit::WebSWServerToContextConnection, WTF::DumbPtrTraits<WebKit::WebSWServerToContextConnection> > > >, WebCore::SecurityOriginDataHash, WTF::HashMap<WebCore::SecurityOriginData, WTF::Ref<WebKit::WebSWServerToContextConnection, WTF::DumbPtrTraits<WebKit::WebSWServerToContextConnection> >, WebCore::SecurityOriginDataHash, WTF::HashTraits<WebCore::SecurityOriginData>, WTF::HashTraits<WTF::Ref<WebKit::WebSWServerToContextConnection, WTF::DumbPtrTraits<WebKit::WebSWServerToContextConnection> > > >::KeyValuePairTraits, WTF::HashTraits<WebCore::SecurityOriginData> > > WTF::HashTable<WebCore::SecurityOriginData, WTF::KeyValuePair<WebCore::SecurityOriginData, WTF::Ref<WebKit::WebSWServerToContextConnection, WTF::DumbPtrTraits<WebKit::WebSWServerToContextConnection> > >, WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<WebCore::SecurityOriginData, WTF::Ref<WebKit::WebSWServerToContextConnection, WTF::DumbPtrTraits<WebKit::WebSWServerToContextConnection> > > >, WebCore::SecurityOriginDataHash, WTF::HashMap<WebCore::SecurityOriginData, WTF::Ref<WebKit::WebSWServerToContextConnection, WTF::DumbPtrTraits<WebKit::WebSWServerToContextConnection> >, WebCore::SecurityOriginDataHash, WTF::HashTraits<WebCore::SecurityOriginData>, WTF::HashTraits<WTF::Ref<WebKit::WebSWServerToContextConnection, WTF::DumbPtrTraits<WebKit::WebSWServerToContextConnection> > > >::KeyValuePairTraits, WTF::HashTraits<WebCore::SecurityOriginData> >::add<WTF::HashMapTranslator<WTF::HashMap<WebCore::SecurityOriginData, WTF::Ref<WebKit::WebSWServerToContextConnection, WTF::DumbPtrTraits<WebKit::WebSWServerToContextConnection> >, WebCore::SecurityOriginDataHash, WTF::HashTraits<WebCore::SecurityOriginData>, WTF::HashTraits<WTF::Ref<WebKit::WebSWServerToContextConnection, WTF::DumbPtrTraits<WebKit::WebSWServerToContextConnection> > > >::KeyValuePairTraits, WebCore::SecurityOriginDataHash>, WebCore::SecurityOriginData const&, WTF::Ref<WebKit::WebSWServerToContextConnection, WTF::DumbPtrTraits<WebKit::WebSWServerToContextConnection> > >(WebCore::SecurityOriginData const&&&, WTF::Ref<WebKit::WebSWServerToContextConnection, WTF::DumbPtrTraits<WebKit::WebSWServerToContextConnection> >&&) + 108 (HashTable.h:869)
13  com.apple.WebKit              	0x000000010f51801c WTF::HashTableAddResult<WTF::HashTableIterator<WebCore::SecurityOriginData, WTF::KeyValuePair<WebCore::SecurityOriginData, WTF::Ref<WebKit::WebSWServerToContextConnection, WTF::DumbPtrTraits<WebKit::WebSWServerToContextConnection> > >, WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<WebCore::SecurityOriginData, WTF::Ref<WebKit::WebSWServerToContextConnection, WTF::DumbPtrTraits<WebKit::WebSWServerToContextConnection> > > >, WebCore::SecurityOriginDataHash, WTF::HashMap<WebCore::SecurityOriginData, WTF::Ref<WebKit::WebSWServerToContextConnection, WTF::DumbPtrTraits<WebKit::WebSWServerToContextConnection> >, WebCore::SecurityOriginDataHash, WTF::HashTraits<WebCore::SecurityOriginData>, WTF::HashTraits<WTF::Ref<WebKit::WebSWServerToContextConnection, WTF::DumbPtrTraits<WebKit::WebSWServerToContextConnection> > > >::KeyValuePairTraits, WTF::HashTraits<WebCore::SecurityOriginData> > > WTF::HashMap<WebCore::SecurityOriginData, WTF::Ref<WebKit::WebSWServerToContextConnection, WTF::DumbPtrTraits<WebKit::WebSWServerToContextConnection> >, WebCore::SecurityOriginDataHash, WTF::HashTraits<WebCore::SecurityOriginData>, WTF::HashTraits<WTF::Ref<WebKit::WebSWServerToContextConnection, WTF::DumbPtrTraits<WebKit::WebSWServerToContextConnection> > > >::inlineAdd<WebCore::SecurityOriginData const&, WTF::Ref<WebKit::WebSWServerToContextConnection, WTF::DumbPtrTraits<WebKit::WebSWServerToContextConnection> > >(WebCore::SecurityOriginData const&&&, WTF::Ref<WebKit::WebSWServerToContextConnection, WTF::DumbPtrTraits<WebKit::WebSWServerToContextConnection> >&&) + 60 (HashMap.h:346)
14  com.apple.WebKit              	0x000000010f505f34 WTF::HashTableAddResult<WTF::HashTableIterator<WebCore::SecurityOriginData, WTF::KeyValuePair<WebCore::SecurityOriginData, WTF::Ref<WebKit::WebSWServerToContextConnection, WTF::DumbPtrTraits<WebKit::WebSWServerToContextConnection> > >, WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<WebCore::SecurityOriginData, WTF::Ref<WebKit::WebSWServerToContextConnection, WTF::DumbPtrTraits<WebKit::WebSWServerToContextConnection> > > >, WebCore::SecurityOriginDataHash, WTF::HashMap<WebCore::SecurityOriginData, WTF::Ref<WebKit::WebSWServerToContextConnection, WTF::DumbPtrTraits<WebKit::WebSWServerToContextConnection> >, WebCore::SecurityOriginDataHash, WTF::HashTraits<WebCore::SecurityOriginData>, WTF::HashTraits<WTF::Ref<WebKit::WebSWServerToContextConnection, WTF::DumbPtrTraits<WebKit::WebSWServerToContextConnection> > > >::KeyValuePairTraits, WTF::HashTraits<WebCore::SecurityOriginData> > > WTF::HashMap<WebCore::SecurityOriginData, WTF::Ref<WebKit::WebSWServerToContextConnection, WTF::DumbPtrTraits<WebKit::WebSWServerToContextConnection> >, WebCore::SecurityOriginDataHash, WTF::HashTraits<WebCore::SecurityOriginData>, WTF::HashTraits<WTF::Ref<WebKit::WebSWServerToContextConnection, WTF::DumbPtrTraits<WebKit::WebSWServerToContextConnection> > > >::add<WTF::Ref<WebKit::WebSWServerToContextConnection, WTF::DumbPtrTraits<WebKit::WebSWServerToContextConnection> > >(WebCore::SecurityOriginData const&, WTF::Ref<WebKit::WebSWServerToContextConnection, WTF::DumbPtrTraits<WebKit::WebSWServerToContextConnection> >&&) + 52 (HashMap.h:381)
15  com.apple.WebKit              	0x000000010f505616 WebKit::StorageProcess::createStorageToWebProcessConnection(bool, WebCore::SecurityOriginData&&) + 726 (StorageProcess.cpp:281)

[Chris Dumez]

HashMap<RefPtr<WebCore::SecurityOrigin>, RefPtr<WebSWServerToContextConnection>> type

[Chris Dumez]

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.

[Chris Dumez]

(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));
        }
    };

[Chris Dumez]

(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?

[Darin Adler]

So we consider it a programming error to move from a Ref that is null. But we want to use null values in the hash table. I think the best way to work around this is in the hash table machinery rather than in Ref; I don’t think we should use a value other than nullptr for empty since I think it would hurt efficiency.

For this specific case, I think we could figure out how to make emptyValueIsZero be true for SecurityOriginData. I am pretty sure that works even for std::optional.

[Chris Dumez]

(In reply to Darin Adler from comment #5)
> So we consider it a programming error to move from a Ref that is null. But
> we want to use null values in the hash table. I think the best way to work
> around this is in the hash table machinery rather than in Ref; I don’t think
> we should use a value other than nullptr for empty since I think it would
> hurt efficiency.
> 
> For this specific case, I think we could figure out how to make
> emptyValueIsZero be true for SecurityOriginData. I am pretty sure that works
> even for std::optional.

Fixing it only for SecurityOriginData would not be optimal IMO. Someone else is going to get caught by this in the future for another key type's whose emptyValueIsZero is false.

[Chris Dumez]

Created attachment 336628 [details]
Patch

[Ryosuke Niwa]

Comment on attachment 336628 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=336628&action=review

> Source/WebCore/ChangeLog:9
> +        Set SecurityOriginDataHashTraits::emptyValueIsZero to true. This is OK because
> +        the data members of SecurityOriginData are of type String or std::optional<uint16_t>.

This seems a bit fragile. Can we add an assertion somewhere to make sure this assumption holds?
e.g. memset SecurityOriginData and make sure isEmpty returns true there.