Bug 223640 - Extend WebAudio heap allocation assertions to cover the pre & post-rendering phases
Summary: Extend WebAudio heap allocation assertions to cover the pre & post-rendering ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Audio (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords: InRadar
Depends on: 223226
Blocks:
  Show dependency tree / graph
 
Reported: 2021-03-23 09:19 PDT by Chris Dumez
Modified: 2021-03-24 17:47 PDT (History)
16 users (show)

See Also:

Attachments
Patch (6.06 KB, patch)
2021-03-23 09:21 PDT, Chris Dumez 		no flags Details | Formatted Diff | Diff
Patch (9.12 KB, patch)
2021-03-23 12:51 PDT, Chris Dumez 		no flags Details | Formatted Diff | Diff
Patch (8.45 KB, patch)
2021-03-23 14:48 PDT, Chris Dumez 		no flags Details | Formatted Diff | Diff
Patch (20.94 KB, patch)
2021-03-23 17:41 PDT, Chris Dumez 		no flags Details | Formatted Diff | Diff
Patch (22.86 KB, patch)
2021-03-24 08:09 PDT, Chris Dumez 		no flags Details | Formatted Diff | Diff
Patch (23.96 KB, patch)
2021-03-24 12:28 PDT, Chris Dumez 		no flags Details | Formatted Diff | Diff
Show Obsolete (5) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Dumez 2021-03-23 09:19:44 PDT 
Extend WebAudio heap allocation assertions to cover the pre-rendering phrase.
Comment 1 Chris Dumez 2021-03-23 09:21:07 PDT 
Created attachment 424024 [details]
Patch
Comment 2 Chris Dumez 2021-03-23 12:51:25 PDT 
Created attachment 424051 [details]
Patch
Comment 3 Chris Dumez 2021-03-23 14:48:55 PDT 
Created attachment 424069 [details]
Patch
Comment 4 Sam Weinig 2021-03-23 17:08:43 PDT 
Comment on attachment 424069 [details]
Patch

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

> Source/WebCore/Modules/webaudio/AudioDestinationNode.cpp:60
> +    ForbidMallocUseForCurrentThreadScope forbidMallocUse;

I think giving a comment for these kind of things would be useful, as to someone unfamiliar with this code / audio thread needs, it is a bit of mystery why malloc would be forbidden, or explain the reason a bit in the variable name.

> Source/WebCore/Modules/webaudio/AudioNodeInput.cpp:106
> +    DisableMallocRestrictionsForCurrentThreadScope disableMallocRestrictions;

Same as above, add a comment or improve the variable name, with why we are allowing malloc here?

> Source/WebCore/Modules/webaudio/AudioNodeOutput.cpp:71
> +    DisableMallocRestrictionsForCurrentThreadScope disableMallocRestrictions;

Same as above, add a comment or improve the variable name, with why we are allowing malloc here?

> Source/WebCore/Modules/webaudio/AudioWorkletGlobalScope.cpp:191
> +        DisableMallocRestrictionsForCurrentThreadScope disableMallocRestrictions;

Same as above, add a comment or improve the variable name, with why we are allowing malloc here?

> Source/WebCore/Modules/webaudio/BaseAudioContext.cpp:874
> +    // FIXME: This may cause heap allocations on the audio thread.
> +    DisableMallocRestrictionsForCurrentThreadScope disableMallocRestrictions;

This FIXME is a bit misleading. DisableMallocRestrictionsForCurrentThreadScope won't *cause* the allocations, it only allows them. Adding an explanation as to why they are being allowed would be useful though. (Same for all the other ones).
Comment 5 Chris Dumez 2021-03-23 17:41:15 PDT 
Created attachment 424082 [details]
Patch
Comment 6 Chris Dumez 2021-03-24 08:09:01 PDT 
Created attachment 424135 [details]
Patch
Comment 7 Chris Dumez 2021-03-24 12:28:41 PDT 
Created attachment 424166 [details]
Patch
Comment 8 EWS 2021-03-24 17:46:44 PDT 
Committed r274989: <https://commits.webkit.org/r274989>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 424166 [details].
Comment 9 Radar WebKit Bug Importer 2021-03-24 17:47:15 PDT 
<rdar://problem/75814355>