Bug 223640

Summary: Extend WebAudio heap allocation assertions to cover the pre & post-rendering phases
Product: WebKit Reporter: Chris Dumez <cdumez>
Component: Web AudioAssignee: Chris Dumez <cdumez>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, cdumez, cmarcelo, darin, eric.carlson, ews-watchlist, ggaren, glenn, hta, jer.noble, peng.liu6, philipj, sam, sergio, tommyw, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 223226    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch none

Chris Dumez
Reported 2021-03-23 09:19:44 PDT
Extend WebAudio heap allocation assertions to cover the pre-rendering phrase.
Attachments
Patch (6.06 KB, patch)
2021-03-23 09:21 PDT, Chris Dumez 		no flags
DetailsFormatted DiffDiff
Patch (9.12 KB, patch)
2021-03-23 12:51 PDT, Chris Dumez 		no flags
DetailsFormatted DiffDiff
Patch (8.45 KB, patch)
2021-03-23 14:48 PDT, Chris Dumez 		no flags
DetailsFormatted DiffDiff
Patch (20.94 KB, patch)
2021-03-23 17:41 PDT, Chris Dumez 		no flags
DetailsFormatted DiffDiff
Patch (22.86 KB, patch)
2021-03-24 08:09 PDT, Chris Dumez 		no flags
DetailsFormatted DiffDiff
Patch (23.96 KB, patch)
2021-03-24 12:28 PDT, Chris Dumez 		no flags
DetailsFormatted DiffDiff
Show Obsolete (5) View All Add attachment proposed patch, testcase, etc.
Chris Dumez
Comment 1 2021-03-23 09:21:07 PDT
Created attachment 424024 [details] Patch
Chris Dumez
Comment 2 2021-03-23 12:51:25 PDT
Created attachment 424051 [details] Patch
Chris Dumez
Comment 3 2021-03-23 14:48:55 PDT
Created attachment 424069 [details] Patch
Sam Weinig
Comment 4 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).
Chris Dumez
Comment 5 2021-03-23 17:41:15 PDT
Created attachment 424082 [details] Patch
Chris Dumez
Comment 6 2021-03-24 08:09:01 PDT
Created attachment 424135 [details] Patch
Chris Dumez
Comment 7 2021-03-24 12:28:41 PDT
Created attachment 424166 [details] Patch
EWS
Comment 8 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].
Radar WebKit Bug Importer
Comment 9 2021-03-24 17:47:15 PDT
<rdar://problem/75814355>
Note You need to log in before you can comment on or make changes to this bug.