Bug 224387 – [ macOS ] 3 webaudio/OfflineAudioContext/ layout-tests are flakey text failures

• Component: New Bugs
• Status: RESOLVED FIXED

[Robert Jenner]

The following three tests are flakey text failures on macOS Catalina and up. 

webaudio/OfflineAudioContext/offlineaudiocontext-leak-after-rendering.html
webaudio/OfflineAudioContext/offlineaudiocontext-leak-while-suspended.html
webaudio/OfflineAudioContext/offlineaudiocontext-leak.html

HISTORY URL:
https://results.webkit.org/?suite=layout-tests&suite=layout-tests&suite=layout-tests&test=webaudio%2FOfflineAudioContext%2Fofflineaudiocontext-leak-after-rendering.html&test=webaudio%2FOfflineAudioContext%2Fofflineaudiocontext-leak-while-suspended.html&test=webaudio%2FOfflineAudioContext%2Fofflineaudiocontext-leak.html

It looks like these tests were started fairly recently, and it appears they've had flakey failures since they started. It also looks like all three tests fail at the same time, and have the same text diff. 

TEXT DIFF:
-PASS internals.numberOfBaseAudioContexts() < instancesToCreate is true
+FAIL internals.numberOfBaseAudioContexts() < instancesToCreate should be true. Was false.
 PASS successfullyParsed is true
+Some tests failed.
 
 TEST COMPLETE

[Radar WebKit Bug Importer]

<rdar://problem/76468058>

[Robert Jenner]

When running these tests standalone, they do not fail anywhere. However, I was finally able to reproduce their failures at BigSur Release ToT using a test list, and the following test:

run-webkit-tests <path to revision being tested> --test-list <path to test list location> --child-process=1

What this indicates is that these tests failures are dependent upon tests that run before them. I have attached the test list I used for reproduction, and will begin narrowing down what causes these tests to fail.

[Robert Jenner]

Created attachment 425651 [details]
224387-Test list

Test list used to reproduce the test failures.

[Robert Jenner]

The following two tests were introduced at r275668(https://trac.webkit.org/changeset/275668/webkit):

webaudio/OfflineAudioContext/offlineaudiocontext-leak.html
webaudio/OfflineAudioContext/offlineaudiocontext-leak-after-rendering.html

The third test was introduced at r275679 (https://trac.webkit.org/changeset/275679/webkit):
webaudio/OfflineAudioContext/offlineaudiocontext-leak-while-suspended.html

[Robert Jenner]

After a process of elimination, I have verified that the following test causes the three tests to fail when ran before them:

webaudio/AudioListener/audiolistener-set-position.html

When I ran the test directly before the three tests in a four test list on a loop, it caused the three tests to fail. When I removed the AudioListener test from the entire 6000+ test list, the three tests passed.

[Chris Dumez]

Investigating now, thanks.

[Chris Dumez]

Can reproduce like so:
run-webkit-tests webaudio/AudioListener/audiolistener-set-position.html webaudio/OfflineAudioContext/offlineaudiocontext-leak.html --child-processes=1 --iterations=30

It does not fail on first iteration but it starts failing after a while. webaudio/AudioListener/audiolistener-set-position.html is probably leaking contexts.

[Chris Dumez]

Looks like webaudio/AudioListener/audiolistener-set-position.html is leaking PannerNodes and the PannerNodes (like all AudioNodes) are ref'ing the AudioContext.

[Chris Dumez]

Created attachment 425663 [details]
Patch

[Chris Dumez]

Created attachment 425665 [details]
Patch

[Peng Liu]

Comment on attachment 425665 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=425665&action=review

> Source/WebCore/testing/Internals.cpp:2659
> +uint64_t Internals::baseAudioContextIdentifier(BaseAudioContext& context)

Nit. const?

[Chris Dumez]

Created attachment 425669 [details]
Patch

[Chris Dumez]

(In reply to Peng Liu from comment #11)
> Comment on attachment 425665 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=425665&action=review
> 
> > Source/WebCore/testing/Internals.cpp:2659
> > +uint64_t Internals::baseAudioContextIdentifier(BaseAudioContext& context)
> 
> Nit. const?

Fixed, thanks.

[Chris Dumez]

Created attachment 425670 [details]
Patch

[Eric Carlson]

Comment on attachment 425670 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=425670&action=review

> Source/WebCore/Modules/webaudio/BaseAudioContext.cpp:123
> +    static uint64_t contextIDSeed = 0;

Not: won’t this be initialized to zero automatically?

[Chris Dumez]

(In reply to Eric Carlson from comment #15)
> Comment on attachment 425670 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=425670&action=review
> 
> > Source/WebCore/Modules/webaudio/BaseAudioContext.cpp:123
> > +    static uint64_t contextIDSeed = 0;
> 
> Not: won’t this be initialized to zero automatically?

Yes, will fix.

[Chris Dumez]

Created attachment 425675 [details]
Patch

[Chris Dumez]

Comment on attachment 425675 [details]
Patch

Looks like the stress bot wasn't happy. Will check.

[EWS]

Committed r275798 (236369@main): <https://commits.webkit.org/236369@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 425675 [details].