Bug 198346 – IndexedDatabase Server thread in com.apple.WebKit.Networking process leaks objects into an autoreleasePool that's never cleared

• Component: WebKit2
• Status: RESOLVED FIXED

[David Kilzer (:ddkilzer)]

IndexedDatabase Server thread in com.apple.WebKit.Networking process leaks objects into an autoreleasePool that's never cleared.

Running a bunch of IndexedDB tests on a single process shows how these objects build up:

$ ./Tools/Scripts/run-webkit-tests --no-build --release --batch-size=1500 --child-processes=1 --leaks --no-retry --no-sample --time-out-ms=60000 --no-show-results LayoutTests/inspector/indexeddb LayoutTests/storage/indexeddb LayoutTests/http/tests/IndexedDB LayoutTests/imported/blink/storage/indexeddb LayoutTests/imported/w3c/IndexedDB-private-browsing LayoutTests/imported/w3c/web-platform-tests/IndexedDB

$ leaks -auto com.apple.WebKit.Networking.Development-58655-UNFIXED.memgraph | less
[…]
Thread 5  Thread_17325843: IndexedDatabase Server
    <@autoreleasepool content 0x7face9815000> [4096]
        0:  <CFString 0x7face8710870> [192]
        1:  <NSPathStore2 0x7face87188f0> [368]
        2:  2 (304 bytes) <NSURL 0x7face871ae10> [96]
               1 (208 bytes) _clients --> <CFString 0x7face87030b0> [208]
        3:  <NSMutableArray 0x7face8717f40> [48]
        4:  <NSArray 0x7face8715ac0> [16]
[…]
        9595:  <NSMutableArray 0x7face865e0f0> [48]
        9596:  <NSArray 0x7face865c9d0> [16]

Thread 6  Thread_17325989: IndexedDatabase Server
    no autorelease pool
[…]

[David Kilzer (:ddkilzer)]

<rdar://problem/50895658>

[David Kilzer (:ddkilzer)]

Created attachment 370877 [details]
Patch v1

[EWS Watchlist]

Attachment 370877 [details] did not pass style-queue:


ERROR: Source/WTF/wtf/CrossThreadTaskHandler.cpp:29:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 1 in 5 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[David Kilzer (:ddkilzer)]

(In reply to Build Bot from comment #3)
> Attachment 370877 [details] did not pass style-queue:
> 
> 
> ERROR: Source/WTF/wtf/CrossThreadTaskHandler.cpp:29:  Alphabetical sorting
> problem.  [build/include_order] [4]
> Total errors found: 1 in 5 files
> 
> 
> If any of these errors are false positives, please file a bug against
> check-webkit-style.

Bug 198349: check-webkit-style reports false-positive build/include_order warning in WTF C++ source files

[Brent Fulgham]

Comment on attachment 370877 [details]
Patch v1

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

> Source/WTF/wtf/CrossThreadTaskHandler.h:44
> +    WTF_EXPORT_PRIVATE CrossThreadTaskHandler(const char* threadName, AutodrainedPoolForRunLoop = AutodrainedPoolForRunLoop::DoNotUse);

Since the only use of CrossThreadTaskHandler is IndexedDB, and IndexedDB needs an auto drained pool, should we just make auto drained the default?

[David Kilzer (:ddkilzer)]

Comment on attachment 370877 [details]
Patch v1

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

>> Source/WTF/wtf/CrossThreadTaskHandler.h:44
>> +    WTF_EXPORT_PRIVATE CrossThreadTaskHandler(const char* threadName, AutodrainedPoolForRunLoop = AutodrainedPoolForRunLoop::DoNotUse);
> 
> Since the only use of CrossThreadTaskHandler is IndexedDB, and IndexedDB needs an auto drained pool, should we just make auto drained the default?

The reason I made AutodrainedPoolForRunLoop::DoNotUse the default is that I think it tends to be unusual to run Objective-C code (on background threads) in WebKit, but maybe I'm wrong?

However, I don't have an objection to making AutodrainedPoolForRunLoop::Use the default since CrossThreadTaskHandler is only used by IndexedDB currently.

I do think we should keep the AutodrainedPoolForRunLoop enum class argument, though, since it make using the class in other scenarios easier, and hopefully makes future users ask the question whether they need an AutodrainedPool or not for their task runloop.

[Brent Fulgham]

Comment on attachment 370877 [details]
Patch v1

Looks good! r=me

[WebKit Commit Bot]

Comment on attachment 370877 [details]
Patch v1

Clearing flags on attachment: 370877

Committed r245871: <https://trac.webkit.org/changeset/245871>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.