Bug 205950 - WebKitTestRunner leaks objects in a top-level autoreleasePool that's never cleared
Summary: WebKitTestRunner leaks objects in a top-level autoreleasePool that's never cl...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: David Kilzer (:ddkilzer)
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-01-08 12:14 PST by David Kilzer (:ddkilzer)
Modified: 2020-01-09 11:35 PST (History)
4 users (show)

See Also:

Attachments
Patch v1 (5.61 KB, patch)
2020-01-08 15:05 PST, David Kilzer (:ddkilzer) 		no flags Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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.