Bug 205950

Summary: WebKitTestRunner leaks objects in a top-level autoreleasePool that's never cleared
Product: WebKit Reporter: David Kilzer (:ddkilzer) <ddkilzer>
Component: Tools / TestsAssignee: David Kilzer (:ddkilzer) <ddkilzer>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, darin, joepeck, 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=206018
Attachments:
Description Flags
Patch v1 none

Description David Kilzer (:ddkilzer) 2020-01-08 12:14:37 PST 
WebKitTestRunner leaks objects in a top-level autoreleasePool that's never cleared.

This occurs on both iOS and macOS.

<rdar://problem/50987831>
Comment 1 David Kilzer (:ddkilzer) 2020-01-08 15:05:56 PST 
Created attachment 387145 [details]
Patch v1
Comment 2 Joseph Pecoraro 2020-01-08 17:10:55 PST 
Comment on attachment 387145 [details]
Patch v1

Nice! r=me
Comment 3 WebKit Commit Bot 2020-01-09 03:47:38 PST 
Comment on attachment 387145 [details]
Patch v1

Clearing flags on attachment: 387145

Committed r254254: <https://trac.webkit.org/changeset/254254>
Comment 4 WebKit Commit Bot 2020-01-09 03:47:40 PST 
All reviewed patches have been landed.  Closing bug.
Comment 5 Darin Adler 2020-01-09 10:36:21 PST 
Comment on attachment 387145 [details]
Patch v1

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

> Tools/WebKitTestRunner/ios/mainIOS.mm:73
>      UIApplicationMain(argc, (char**)argv, @"WebKitTestRunnerApp", @"WebKitTestRunnerApp");

No need for an autorelease pool around this?

> Tools/WebKitTestRunner/mac/main.mm:68
> +    WTR::TestController controller(argc, argv);

No need for an autorelease pool around this?
Comment 6 Darin Adler 2020-01-09 10:38:00 PST 
Comment on attachment 387145 [details]
Patch v1

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

>> Tools/WebKitTestRunner/ios/mainIOS.mm:73
>>      UIApplicationMain(argc, (char**)argv, @"WebKitTestRunnerApp", @"WebKitTestRunnerApp");
> 
> No need for an autorelease pool around this?

I guess it would never be drained, so it would be bad if it was needed, but I’m still wonder if we might start getting diagnostics about things being autoreleased without a pool. Although those would be leaks, I guess.

>> Tools/WebKitTestRunner/mac/main.mm:68
>> +    WTR::TestController controller(argc, argv);
> 
> No need for an autorelease pool around this?

Same.
Comment 7 David Kilzer (:ddkilzer) 2020-01-09 10:45:12 PST 
Comment on attachment 387145 [details]
Patch v1

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

>>> Tools/WebKitTestRunner/ios/mainIOS.mm:73
>>>      UIApplicationMain(argc, (char**)argv, @"WebKitTestRunnerApp", @"WebKitTestRunnerApp");
>> 
>> No need for an autorelease pool around this?
> 
> I guess it would never be drained, so it would be bad if it was needed, but I’m still wonder if we might start getting diagnostics about things being autoreleased without a pool. Although those would be leaks, I guess.

Any autoreleasePool "leaks" inside UIApplicationMain() need to be handled in a localized manner.

As you note, adding an @autoreleasepool block around UIApplicationMain() would never be drained.

>>> Tools/WebKitTestRunner/mac/main.mm:68
>>> +    WTR::TestController controller(argc, argv);
>> 
>> No need for an autorelease pool around this?
> 
> Same.

Same, but there are some additional autoreleasePool "leaks" under WTR::TestController::TestController() during initialization, so a follow-up patch is warranted.
Comment 8 David Kilzer (:ddkilzer) 2020-01-09 11:35:12 PST 
Additional improvements to autoreleasePool leaks tracked in Bug 206018.