Bug 236277

Summary: [WebCore] JSValueInWrappedObject is not correct for concurrent GC
Product: WebKit Reporter: Yusuke Suzuki <ysuzuki>
Component: New BugsAssignee: Yusuke Suzuki <ysuzuki>
Status: RESOLVED FIXED    
Severity: Normal CC: alecflett, beidson, eric.carlson, ews-watchlist, glenn, jer.noble, jsbell, philipj, saam, sergio, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=236278
https://bugs.webkit.org/show_bug.cgi?id=236279
Bug Depends on: 194806    
Bug Blocks:    
Attachments:
Description Flags
Patch
saam: review+
Patch saam: commit-queue-

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>