[webkit-reviews] review granted: [Bug 112762] Add an assertion to ensure AtomicString from two different threads are not compared : [Attachment 205639] Patch

[bugzilla-daemon at webkit.org]

Alexey Proskuryakov <ap at webkit.org> has granted Benjamin Poulain
<benjamin at webkit.org>'s request for review:
Bug 112762: Add an assertion to ensure AtomicString from two different threads
are not compared
https://bugs.webkit.org/show_bug.cgi?id=112762

Attachment 205639: Patch
https://bugs.webkit.org/attachment.cgi?id=205639&action=review

------- Additional Comments from Alexey Proskuryakov <ap at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=205639&action=review


> Source/WTF/wtf/text/AtomicString.cpp:431
> +    r->setAtomicStringTable(0);

Perhaps not part of this patch, but we really need to assert that we are not
removing from the wrong table here.

> Source/WTF/wtf/text/AtomicStringImpl.h:48
> +ALWAYS_INLINE bool safeAtomicEqual(const AtomicStringImpl* a, const
AtomicStringImpl* b)

I'm not a fan of "safe" names, they don't really explain much. Perhaps
"equalWithThreadingAssertion"? Or just split the assertion into a separate
function, and have callers invoke it explicitly before comparing pointers?

The latter seems preferable to me.