Safari crashes after ~2028 OfflineAudioContext objects are created (they never get garbage collected, consuming a thread each)

• Component: Web Audio
• Status: RESOLVED FIXED

[schngrg]

Trying to create more than 2028 OfflineAudioContext (or AudioContext) crashes the Safari/WebKit page. In WebKit C++ code (AudioContext.cpp), it looks like every OfflineAudioContext takes a thread and the crash is forced when the total pool of 2048 threads is exhausted.

Possible solution: Fix garbage collection of OfflineAudioContexts. OfflineAudioContexts are never garbage collected. In WebKit C++ code, any code that could mark OfflineAudioContext as ready for garbage collection is not reached because of isOfflineContext() checks in AudioContext.cpp.

Background: WebPage gets small buffers of audio data, and wants to load it in audiobuffers to process with OfflineAudioContext. OfflineAudioContext can't be reused (https://github.com/WebAudio/web-audio-api/issues/346). For each small buffer of audio data received a new OfflineAudioContext needs to be created, but the total limit is soon exhausted because OfflineAudioContext never gets garbage collected.

Tried calling both stop() or close() etc but that didn't help because of isOfflineContext() checks in AudioContext.cpp.


Sample code:
    for (let i = 0; i < 3000; i++) { // Safari will crash at i 2026 or 2028, on both iOS and Mac
      let ctx = new OfflineAudioContext(1, 16384, 44100);
      console.log(i);
    }

[Radar WebKit Bug Importer]

<rdar://problem/51891520>

[schngrg]

Above for loop is only test code to reproduce the problem quickly. The actual application was receiving new audio data every 100ms, loading it in a AudioBuffer to use with AudioBufferSourceNode.start() and calling OfflineAudioContext.startRendering(). The oncomplete callback is successfully called. 

But nothing gets garbage collected (all the buffers etc stay), making it look like a memory leak. But as a hack, setting oncomplete = null after oncomplete is called causes garbage collection of everything except OfflineAudioContext objects. Very little memory is consumed by the OfflineAudioContext objects, < 40KB total. But OfflineAudioContext objects seem to be holding on to the threads causing a crash as soon as around 2020 to 2035 objects are created.

Trying to call AudioBufferSourceNode.stop(), AudioBufferSourceNode.disconnect(), setting all web-audio objects to null in or after oncomplete, etc doesn't help.

[youenn fablet]

I cannot reproduce the crash on latest MacOS.

Thread increase is probably due to AsyncAudioDecoder.
It does not seem great to create a decoding thread per AudioContext, since it might never be used in practice. At least this should be lazily initialized.

Agreed OfflineAudioContext should be easily GCable, at least once it has finished its task.

[schngrg]

I can reproduce the crash using Safari Technology Preview 85 on macOS. Using the for loop in my first post that just creates 3000 OfflineAudioContexts.

Note that same crash occurs even with the normal AudioContext, but it might be less of a problem with AudioContext as that is very unlikely to get created so many times.

GC is probably blocked by the setPendingActivity call in AudioContext::constructCommon in C++ code? (I am very new to webkit code, just built it for first time few days ago to try and figure out this crash).

[youenn fablet]

> GC is probably blocked by the setPendingActivity call in
> AudioContext::constructCommon in C++ code? (I am very new to webkit code,
> just built it for first time few days ago to try and figure out this crash).

Welcome to WebKit!
And yep, that is probably the issue.

[youenn fablet]

Created attachment 372487 [details]
Patch

[youenn fablet]

(In reply to youenn fablet from comment #6)
> Created attachment 372487 [details]
> Patch

This patch delays setting pending activity of an OfflineAudioContext until startsRendering is called. I think this is ok but might need some additional study.

In case finishedRendering exits in case of renderedBuffer is null, there is a chance the OfflineAudioContext gets GCed while a state change event might be enqueued. This seems ok to me as renderedBuffer is not supposed to be null for offline audio contexts.

[schngrg]

Thanks :-)

[EWS Watchlist]

Comment on attachment 372487 [details]
Patch

Attachment 372487 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/12523687

New failing tests:
webaudio/mediaelementaudiosourcenode-gc.html
webaudio/offlineaudiocontext-gc.html

[EWS Watchlist]

Created attachment 372492 [details]
Archive of layout-test-results from ews103 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-highsierra  Platform: Mac OS X 10.13.6

[youenn fablet]

Created attachment 372493 [details]
Patch

[youenn fablet]

Created attachment 372494 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 372494 [details]
Patch

Clearing flags on attachment: 372494

Committed r246685: <https://trac.webkit.org/changeset/246685>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.