Summary: | REGRESSION (r286603): rx::ProgramMtl::getSpecializedShader() leaks MTLFunctionConstantValues object due early return when an error occurs in ANGLE_TRY() macro | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | David Kilzer (:ddkilzer) <ddkilzer> | ||||||
Component: | ANGLE | Assignee: | David Kilzer (:ddkilzer) <ddkilzer> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | darin, dino, ews-watchlist, geofflang, gman, jonahr, kbr, kkinnunen, kondapallykalyan, kpiddington, webkit-bug-importer | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | Other | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
See Also: | https://bugs.webkit.org/show_bug.cgi?id=235282 | ||||||||
Bug Depends on: | 220896, 229128 | ||||||||
Bug Blocks: | |||||||||
Attachments: |
|
Description
David Kilzer (:ddkilzer)
2022-01-16 13:59:51 PST
(In reply to David Kilzer (:ddkilzer) from comment #0) > Regressed with this commit: > > Roll ANGLE to include upstreamed Metal backend > <https://bugs.webkit.org/show_bug.cgi?id=220896> > <rdar://problem/73539682> > <https://commits.webkit.org/r286603> Huh, that commit actually reverted some changes I made in r281160 to fix this leak earlier! [Metal ANGLE] Fix over-autorelease of rx::DisplayMtl::getMetalDeviceMatchingAttribute() and various Objective-C leak s <https://webkit.org/b/229128> <rdar://problem/81964007> <https://commits.webkit.org/r281160> Created attachment 449288 [details]
Patch v1
Note that there are important steps to take when updating ANGLE. See https://trac.webkit.org/wiki/UpdatingANGLE Comment on attachment 449288 [details] Patch v1 View in context: https://bugs.webkit.org/attachment.cgi?id=449288&action=review We can all agree that autorelease is the worst > Source/ThirdParty/ANGLE/src/libANGLE/renderer/metal/ProgramMtl.mm:643 > + [funcConstants.get() setConstantValue:&emulateDiscard I don’t think this .get() is needed, same for the 5 more below, unless mtl::AutoObjCObj is different from RetainPtr in this respect. Created attachment 449341 [details]
Patch for landing
Comment on attachment 449288 [details] Patch v1 View in context: https://bugs.webkit.org/attachment.cgi?id=449288&action=review >> Source/ThirdParty/ANGLE/src/libANGLE/renderer/metal/ProgramMtl.mm:643 >> + [funcConstants.get() setConstantValue:&emulateDiscard > > I don’t think this .get() is needed, same for the 5 more below, unless mtl::AutoObjCObj is different from RetainPtr in this respect. Yes! Will remove and land a follow-up fix for Bug 235278. Committed r288105 (246119@main): <https://commits.webkit.org/246119@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 449341 [details]. David, Darin - FYI Kyle, myself and several other colleagues are actively working on resolving divergence between WebKit's copy of ANGLE, and upstream ANGLE. Filed: Upstream fix for MTLFunctionConstantValues leak in rx::ProgramMtl::getSpecializedShader() https://bugs.chromium.org/p/angleproject/issues/detail?id=6920 |