Bug 235278 - REGRESSION (r286603): Leak of three NSMutableDictionary objects each time rx::DisplayMtl::getMetalDeviceMatchingAttribute() is called
Summary: REGRESSION (r286603): Leak of three NSMutableDictionary objects each time rx:...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: ANGLE (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: David Kilzer (:ddkilzer)
URL:
Keywords: InRadar
Depends on: 220896
Blocks:
  Show dependency treegraph
 
Reported: 2022-01-16 08:57 PST by David Kilzer (:ddkilzer)
Modified: 2022-04-05 12:22 PDT (History)
8 users (show)

See Also:


Attachments
Patch v1 (3.92 KB, patch)
2022-01-16 09:46 PST, David Kilzer (:ddkilzer)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description David Kilzer (:ddkilzer) 2022-01-16 08:57:30 PST
Leak of three NSMutableDictionary objects each time rx::DisplayMtl::getMetalDeviceMatchingAttribute() is called.

These three objects are created each time the method is called, but none of them are released before the method returns:

    NSMutableArray<id<MTLDevice>> *externalGPUs   = [[NSMutableArray alloc] init];
    NSMutableArray<id<MTLDevice>> *integratedGPUs = [[NSMutableArray alloc] init];
    NSMutableArray<id<MTLDevice>> *discreteGPUs   = [[NSMutableArray alloc] init];

Found by clang static analyzer.

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>
Comment 1 Radar WebKit Bug Importer 2022-01-16 08:57:54 PST
<rdar://problem/87653812>
Comment 2 David Kilzer (:ddkilzer) 2022-01-16 09:46:17 PST
Created attachment 449282 [details]
Patch v1
Comment 3 EWS Watchlist 2022-01-16 09:47:24 PST
Note that there are important steps to take when updating ANGLE. See https://trac.webkit.org/wiki/UpdatingANGLE
Comment 4 Darin Adler 2022-01-16 11:05:20 PST
Can ANGLE switch to ARC?
Comment 5 David Kilzer (:ddkilzer) 2022-01-16 13:29:55 PST
(In reply to Darin Adler from comment #4)
> Can ANGLE switch to ARC?

Yes.  In fact, it would probably better align with upstream because I'm pretty sure Google builds ANGLE with ARC enabled today.
Comment 6 Darin Adler 2022-01-16 13:33:45 PST
(In reply to David Kilzer (:ddkilzer) from comment #5)
> (In reply to Darin Adler from comment #4)
> > Can ANGLE switch to ARC?
> 
> Yes.  In fact, it would probably better align with upstream because I'm
> pretty sure Google builds ANGLE with ARC enabled today.

Let’s make that change, then, as soon as practical. I’d be happy to make the change and have you review, or you could make the change and I’d be happy to review.
Comment 7 EWS 2022-01-16 13:35:23 PST
Committed r288074 (246094@main): <https://commits.webkit.org/246094@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 449282 [details].
Comment 8 David Kilzer (:ddkilzer) 2022-01-16 14:04:46 PST
(In reply to Darin Adler from comment #6)
> (In reply to David Kilzer (:ddkilzer) from comment #5)
> > (In reply to Darin Adler from comment #4)
> > > Can ANGLE switch to ARC?
> > 
> > Yes.  In fact, it would probably better align with upstream because I'm
> > pretty sure Google builds ANGLE with ARC enabled today.
> 
> Let’s make that change, then, as soon as practical. I’d be happy to make the
> change and have you review, or you could make the change and I’d be happy to
> review.

I'm not 100% sure about upstream building with ARC now.  Filed this to track:

Bug 235282: ANGLE should build with ARC enabled
Comment 9 David Kilzer (:ddkilzer) 2022-01-17 10:30:52 PST
(In reply to EWS from comment #7)
> Committed r288074 (246094@main): <https://commits.webkit.org/246094@main>
> 
> All reviewed patches have been landed. Closing bug and clearing flags on
> attachment 449282 [details].

Follow-up fix based on Bug 235281, Comment #5:

Committed r288097: <https://commits.webkit.org/r288097>