Bug 236277 - [WebCore] JSValueInWrappedObject is not correct for concurrent GC
Summary: [WebCore] JSValueInWrappedObject is not correct for concurrent GC
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Yusuke Suzuki
URL:
Keywords: InRadar
Depends on: 194806
Blocks:
  Show dependency tree / graph
 
Reported: 2022-02-07 19:12 PST by Yusuke Suzuki
Modified: 2022-02-08 09:53 PST (History)
11 users (show)

See Also:

Attachments
Patch (16.25 KB, patch)
2022-02-07 19:26 PST, Yusuke Suzuki 		saam: review+
Details | Formatted Diff | Diff
Patch (16.95 KB, patch)
2022-02-07 21:49 PST, Yusuke Suzuki 		saam: commit-queue-
Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Yusuke Suzuki 2022-02-07 19:12:53 PST 
[WebCore] JSValueInWrappedObject is not correct for concurrent GC
Comment 1 Yusuke Suzuki 2022-02-07 19:26:11 PST 
Created attachment 451196 [details]
Patch
Comment 2 Yusuke Suzuki 2022-02-07 19:26:14 PST 
<rdar://problem/88366849>
Comment 3 Saam Barati 2022-02-07 19:32:58 PST 
Comment on attachment 451196 [details]
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
Comment 4 Yusuke Suzuki 2022-02-07 19:44:01 PST 
Comment on attachment 451196 [details]
Patch

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

Thanks!

>> Source/WebCore/Modules/indexeddb/IDBRequest.cpp:506
>> +    // if we would like to keep them alive.
> 
> can you file a bug for this and ping the relevant folks?

Nice, filed. https://bugs.webkit.org/show_bug.cgi?id=236278
I will paste the above bugzilla URL here too.

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

Filed :) https://bugs.webkit.org/show_bug.cgi?id=236279

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

Filed :)
Comment 5 Yusuke Suzuki 2022-02-07 21:49:28 PST 
Created attachment 451209 [details]
Patch
Comment 6 Yusuke Suzuki 2022-02-08 09:53:49 PST 
Committed r289383 (?): <https://commits.webkit.org/r289383>