[webkit-reviews] review granted: [Bug 236277] [WebCore] JSValueInWrappedObject is not correct for concurrent GC : [Attachment 451196] Patch

[bugzilla-daemon at webkit.org]

Saam Barati <sbarati at apple.com> has granted Yusuke Suzuki <ysuzuki at apple.com>'s
request for review:
Bug 236277: [WebCore] JSValueInWrappedObject is not correct for concurrent GC
https://bugs.webkit.org/show_bug.cgi?id=236277

Attachment 451196: Patch

https://bugs.webkit.org/attachment.cgi?id=451196&action=review




--- Comment #3 from Saam Barati <sbarati at apple.com> ---
Comment on attachment 451196
  --> https://bugs.webkit.org/attachment.cgi?id=451196
Patch

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

r=me

> Source/WebCore/Modules/indexeddb/IDBRequest.cpp:506
> +    // FIXME: This code is wrong: let's consider that these fields' access
are reordered in the concurrent GC thread.
> +    // And we just scanned cleared m_resultWrapper and then, we missed
scanning m_cursorWrapper with a new value.
> +    // Then we could make both collected. Whenever changing
JSValueInWrappedObject fields, we should emit a write barrier
> +    // if we would like to keep them alive.

can you file a bug for this and ping the relevant folks?

> Source/WebCore/bindings/js/JSValueInWrappedObject.h:42
> +    // Remove them once AudioBuffer's m_channelWrappers bug is fixed.

add FIXME and bug link

> Source/WebCore/bindings/js/JSValueInWrappedObject.h:54
> +    // Remove this once IDBRequest semantic bug is fixed.

ditto about FIXME and bug link