WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
198346
IndexedDatabase Server thread in com.apple.WebKit.Networking process leaks objects into an autoreleasePool that's never cleared
https://bugs.webkit.org/show_bug.cgi?id=198346
Summary
IndexedDatabase Server thread in com.apple.WebKit.Networking process leaks ob...
David Kilzer (:ddkilzer)
Reported
2019-05-29 12:59:49 PDT
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 […]
Attachments
Patch v1
(6.02 KB, patch)
2019-05-29 13:12 PDT
,
David Kilzer (:ddkilzer)
no flags
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
David Kilzer (:ddkilzer)
Comment 1
2019-05-29 13:00:55 PDT
<
rdar://problem/50895658
>
David Kilzer (:ddkilzer)
Comment 2
2019-05-29 13:12:44 PDT
Created
attachment 370877
[details]
Patch v1
EWS Watchlist
Comment 3
2019-05-29 13:14:32 PDT
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)
Comment 4
2019-05-29 13:23:58 PDT
(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 5
2019-05-29 14:15:00 PDT
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 6
2019-05-29 14:22:19 PDT
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 7
2019-05-29 14:40:21 PDT
Comment on
attachment 370877
[details]
Patch v1 Looks good! r=me
WebKit Commit Bot
Comment 8
2019-05-29 15:07:30 PDT
Comment on
attachment 370877
[details]
Patch v1 Clearing flags on attachment: 370877 Committed
r245871
: <
https://trac.webkit.org/changeset/245871
>
WebKit Commit Bot
Comment 9
2019-05-29 15:07:31 PDT
All reviewed patches have been landed. Closing bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug