Bug 216555 – Improve thread-safety in ScriptProcessorNode implementation

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Audio

[Chris Dumez]

Improve thread-safety in ScriptProcessorNode implementation.

[Chris Dumez]

Created attachment 408822 [details]
Patch

[Geoffrey Garen]

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 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]

Created attachment 408830 [details]
Patch

[Chris Dumez]

I switched the code to using a Ref<> instead of explicit ref/deref, as suggested.

[Geoffrey Garen]

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]

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]

<rdar://problem/68934215>