WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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
Details
Formatted Diff
Diff
Patch
(7.75 KB, patch)
2020-09-15 10:34 PDT
,
Chris Dumez
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Chris Dumez
Comment 1
2020-09-15 09:10:01 PDT
Created
attachment 408822
[details]
Patch
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
Created
attachment 408830
[details]
Patch
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
<
rdar://problem/68934215
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug