RESOLVED FIXED 216555
Improve thread-safety in ScriptProcessorNode implementation
https://bugs.webkit.org/show_bug.cgi?id=216555
Summary Improve thread-safety in ScriptProcessorNode implementation
Chris Dumez
Reported 2020-09-15 09:07:42 PDT
Improve thread-safety in ScriptProcessorNode implementation.
Attachments
Patch (7.86 KB, patch)
2020-09-15 09:10 PDT, Chris Dumez
no flags
Patch (7.75 KB, patch)
2020-09-15 10:34 PDT, Chris Dumez
no flags
Chris Dumez
Comment 1 2020-09-15 09:10:01 PDT
Geoffrey Garen
Comment 2 2020-09-15 10:01:41 PDT
Comment on attachment 408822 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=408822&action=review > Source/WebCore/Modules/webaudio/ScriptProcessorNode.cpp:189 > + // Reference ourself so we don't accidentally get deleted before fireProcessEvent() gets called. > + ref(); Can this just be a RefPtr? Also: How is it OK that the main thread can ref/deref at any time? I don't see anything that would make that reference count correct.
Chris Dumez
Comment 3 2020-09-15 10:04:26 PDT
Comment on attachment 408822 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=408822&action=review >> Source/WebCore/Modules/webaudio/ScriptProcessorNode.cpp:189 >> + ref(); > > Can this just be a RefPtr? > > Also: How is it OK that the main thread can ref/deref at any time? I don't see anything that would make that reference count correct. This is not new code. I will check, we may be able to use RefPtr.
Chris Dumez
Comment 4 2020-09-15 10:34:00 PDT
Chris Dumez
Comment 5 2020-09-15 10:34:44 PDT
I switched the code to using a Ref<> instead of explicit ref/deref, as suggested.
Geoffrey Garen
Comment 6 2020-09-15 11:31:08 PDT
Comment on attachment 408830 [details] Patch r=me This seems like an improvement; but I'm pretty sure this code is still not thread-safe.
EWS
Comment 7 2020-09-15 11:57:18 PDT
Committed r267094: <https://trac.webkit.org/changeset/267094> All reviewed patches have been landed. Closing bug and clearing flags on attachment 408830 [details].
Radar WebKit Bug Importer
Comment 8 2020-09-15 11:58:16 PDT
Note You need to log in before you can comment on or make changes to this bug.