Summary: | Improve thread-safety in ScriptProcessorNode implementation | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Chris Dumez <cdumez> | ||||||
Component: | Web Audio | Assignee: | Chris Dumez <cdumez> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | darin, eric.carlson, ews-watchlist, ggaren, glenn, jer.noble, philipj, sergio, webkit-bug-importer | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | WebKit Nightly Build | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Bug Depends on: | |||||||||
Bug Blocks: | 212611 | ||||||||
Attachments: |
|
Description
Chris Dumez
2020-09-15 09:07:42 PDT
Created attachment 408822 [details]
Patch
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. 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. Created attachment 408830 [details]
Patch
I switched the code to using a Ref<> instead of explicit ref/deref, as suggested. 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.
Committed r267094: <https://trac.webkit.org/changeset/267094> All reviewed patches have been landed. Closing bug and clearing flags on attachment 408830 [details]. |