Bug 210710

Summary: REGRESSION (r244091): Leak of TaskStateChangedCallbackType due to missing -dealloc
Product: WebKit Reporter: David Kilzer (:ddkilzer) <ddkilzer>
Component: MediaAssignee: David Kilzer (:ddkilzer) <ddkilzer>
Status: RESOLVED FIXED    
Severity: Normal CC: darin, eric.carlson, jer.noble, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: Other   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 196659    
Bug Blocks:    
Attachments:
Description Flags
Patch v1
none
Patch for landing none

Description David Kilzer (:ddkilzer) 2020-04-18 19:26:57 PDT 
Leak of TaskStateChangedCallbackType due to missing -dealloc.

Found by clang static analyzer:

'WKProcessTaskStateObserverDelegate' lacks a 'dealloc' instance method but must release '_taskStateChangedCallback'

Regressed with the fix for Bug 196659 in r244091:

Bug 196659: [Cocoa] Awaken UIProcess if WebContent process is awakened from suspensions unexpectedly.
<https://bugs.webkit.org/show_bug.cgi?id=196659>
<rdar://problem/49745738>
<https://trac.webkit.org/r244091>
Comment 1 Radar WebKit Bug Importer 2020-04-18 19:27:31 PDT 
<rdar://problem/61992856>
Comment 2 David Kilzer (:ddkilzer) 2020-04-18 19:36:02 PDT 
Created attachment 396887 [details]
Patch v1
Comment 3 David Kilzer (:ddkilzer) 2020-04-18 19:37:25 PDT 
Comment on attachment 396887 [details]
Patch v1

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

> Source/WebKit/Shared/Cocoa/ProcessTaskStateObserver.mm:46
> +- (void)dealloc

I chose to implement dealloc because using RetainPtr<> would require an instance variable and a custom getter (and maybe a setter).

When switching to ARC, we simply delete the -dealloc method and the @property will work as expected.
Comment 4 Darin Adler 2020-04-19 18:07:46 PDT 
Comment on attachment 396887 [details]
Patch v1

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

> Source/WebKit/Shared/Cocoa/ProcessTaskStateObserver.mm:49
> +    _taskStateChangedCallback = nil;

Seems unnecessary to set things to nil in dealloc.
Comment 5 David Kilzer (:ddkilzer) 2020-04-19 19:08:06 PDT 
Created attachment 396935 [details]
Patch for landing
Comment 6 EWS 2020-04-19 20:46:44 PDT 
Committed r260348: <https://trac.webkit.org/changeset/260348>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 396935 [details].