Summary: | REGRESSION (r233409): Leak of NSMapTable in -[JSVirtualMachine addManagedReference:withOwner:] | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | David Kilzer (:ddkilzer) <ddkilzer> | ||||
Component: | JavaScriptCore | Assignee: | David Kilzer (:ddkilzer) <ddkilzer> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | commit-queue, darin, ews-watchlist, joepeck, keith_miller, mark.lam, mitz, msaboff, saam, tzagallo, webkit-bug-importer | ||||
Priority: | P2 | Keywords: | InRadar | ||||
Version: | WebKit Nightly Build | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
See Also: | https://bugs.webkit.org/show_bug.cgi?id=186973 | ||||||
Attachments: |
|
Description
David Kilzer (:ddkilzer)
2019-09-15 06:11:07 PDT
Created attachment 378810 [details]
Patch v1
Wow that was a mismerge. I had a patch that actually converted to ARC, and when making the "prepare to use ARC" version I accidentally left in the deletion of the call to release. It would have been simpler to just add the release back in, but it's also OK to use RetainPtr. Comment on attachment 378810 [details] Patch v1 Clearing flags on attachment: 378810 Committed r249885: <https://trac.webkit.org/changeset/249885> All reviewed patches have been landed. Closing bug. Seriously, could have just added back the release, but rewrite to use RetainPtr was OK. (In reply to Darin Adler from comment #6) > Seriously, could have just added back the release, but rewrite to use > RetainPtr was OK. I didn't add back the -release because: - It would need to be removed to enable ARC again. - Extra insight is required to realize `ownedObjects` isn't being over-released (and is thus still valid) because it was added to m_externalObjectGraph. I'm not sure if the clang static analyzer would have enough context to "know" this. - RetainPtr<> is basically doing the same thing ARC would have done W.r.t. retain counting `ownedObjects`. - I assume that folks will make additional passes through WebKit to remove the use of unnecessary RetainPtr<> for NS objects once the conversion to ARC occurs (and isn't rolled out). |