Bug 200540

Summary: Make IDBRequest ThreadSafeRefCounted
Product: WebKit Reporter: Chris Dumez <cdumez>
Component: WebCore Misc.Assignee: Chris Dumez <cdumez>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, alecflett, beidson, commit-queue, darin, ews-watchlist, fujii.hironori, ggaren, jsbell, rniwa, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 200507    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch none

Chris Dumez
Reported 2019-08-08 10:24:26 PDT
Make IDBRequest ThreadSafeRefCounted, as it looks like it is being ref'd / deref'd from several threads: Exception Type: EXC_BREAKPOINT (SIGTRAP) Exception Codes: 0x0000000000000002, 0x0000000000000000 Exception Note: EXC_CORPSE_NOTIFY Termination Signal: Trace/BPT trap: 5 Termination Reason: Namespace SIGNAL, Code 0x5 Terminating Process: exc handler [0] Application Specific Information: CRASHING TEST: storage/indexeddb/pending-version-change-stuck-private.html Thread 0 Crashed:: Dispatch queue: com.apple.main-thread 0 com.apple.WebCore 0x0000000113674fb3 0x113673000 + 8115 1 com.apple.WebCore 0x0000000113f18f90 WTF::RefPtr<WebCore::IDBOpenDBRequest, WTF::DumbPtrTraits<WebCore::IDBOpenDBRequest> >::operator=(WebCore::IDBOpenDBRequest*) + 224 (Assertions.h:587) 2 com.apple.WebCore 0x0000000113f18dcb WebCore::IDBClient::IDBConnectionProxy::notifyOpenDBRequestBlocked(WebCore::IDBResourceIdentifier const&, unsigned long long, unsigned long long) + 91 (Atomics.h:247) 3 com.apple.JavaScriptCore 0x000000010f59652c WTF::RunLoop::performWork() + 236 (Function.h:79) 4 com.apple.JavaScriptCore 0x000000010f5967d2 WTF::RunLoop::performWork(void*) + 34 (RunLoopCF.cpp:39) 5 com.apple.CoreFoundation 0x00007fff4ca6b581 __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ + 17 6 com.apple.CoreFoundation 0x00007fff4cb238ac __CFRunLoopDoSource0 + 108 7 com.apple.CoreFoundation 0x00007fff4ca4e530 __CFRunLoopDoSources0 + 208 8 com.apple.CoreFoundation 0x00007fff4ca4d9ad __CFRunLoopRun + 1293 9 com.apple.CoreFoundation 0x00007fff4ca4d207 CFRunLoopRunSpecific + 487 10 DumpRenderTree 0x000000010f2b282f runTest(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&) + 3601 (DumpRenderTree.mm:2087) 11 DumpRenderTree 0x000000010f2b165a dumpRenderTree(int, char const**) + 3439 (DumpRenderTree.mm:1210) 12 DumpRenderTree 0x000000010f2b30dd DumpRenderTreeMain(int, char const**) + 1456 (DumpRenderTree.mm:1442) 13 libdyld.dylib 0x00007fff74a0d015 start + 1
Attachments
Patch (8.60 KB, patch)
2019-08-08 10:26 PDT, Chris Dumez 		no flags
DetailsFormatted DiffDiff
Patch (1.60 KB, patch)
2019-08-08 10:29 PDT, Chris Dumez 		no flags
DetailsFormatted DiffDiff
Patch (2.58 KB, patch)
2019-08-08 10:40 PDT, Chris Dumez 		no flags
DetailsFormatted DiffDiff
Patch (2.58 KB, patch)
2019-08-08 10:44 PDT, Chris Dumez 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Chris Dumez
Comment 1 2019-08-08 10:26:25 PDT
Created attachment 375813 [details] Patch
Alex Christensen
Comment 2 2019-08-08 10:27:15 PDT
Comment on attachment 375813 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=375813&action=review > Source/WebKit/WebKit.xcodeproj/project.pbxproj:-8970 > - CDA93DAE22F8BCF300490A69 /* FullscreenTouchSecheuristicParameters.h */, Is all this stuff really necessary with this patch?
Chris Dumez
Comment 3 2019-08-08 10:28:02 PDT
Comment on attachment 375813 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=375813&action=review >> Source/WebKit/WebKit.xcodeproj/project.pbxproj:-8970 >> - CDA93DAE22F8BCF300490A69 /* FullscreenTouchSecheuristicParameters.h */, > > Is all this stuff really necessary with this patch? Not sure what happened there, I'll drop.
Chris Dumez
Comment 4 2019-08-08 10:29:11 PDT
Created attachment 375814 [details] Patch
Chris Dumez
Comment 5 2019-08-08 10:40:11 PDT
Created attachment 375817 [details] Patch
Chris Dumez
Comment 6 2019-08-08 10:44:26 PDT
Created attachment 375818 [details] Patch
Chris Dumez
Comment 7 2019-08-08 11:41:28 PDT
Comment on attachment 375818 [details] Patch Clearing flags on attachment: 375818 Committed r248431: <https://trac.webkit.org/changeset/248431>
Chris Dumez
Comment 8 2019-08-08 11:41:30 PDT
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 9 2019-08-08 11:43:30 PDT
<rdar://problem/54089344>
Darin Adler
Comment 10 2019-08-08 12:52:37 PDT
Comment on attachment 375818 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=375818&action=review > Source/WebCore/Modules/indexeddb/IDBRequest.h:165 > + void refEventTarget() final { ThreadSafeRefCounted::ref(); } > + void derefEventTarget() final { ThreadSafeRefCounted::deref(); } I think we usually just write ref() and deref() here; works because of the using above.
Chris Dumez
Comment 11 2019-08-08 12:58:44 PDT
(In reply to Darin Adler from comment #10) > Comment on attachment 375818 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=375818&action=review > > > Source/WebCore/Modules/indexeddb/IDBRequest.h:165 > > + void refEventTarget() final { ThreadSafeRefCounted::ref(); } > > + void derefEventTarget() final { ThreadSafeRefCounted::deref(); } > > I think we usually just write ref() and deref() here; works because of the > using above. Indeed, fixed in <https://trac.webkit.org/changeset/248439>, thanks.
Fujii Hironori
Comment 12 2019-08-08 21:10:43 PDT
It seems that this change fixed WinCairo flaky indexeddb test failures. Thank you!
Chris Dumez
Comment 13 2019-08-08 21:13:00 PDT
(In reply to Fujii Hironori from comment #12) > It seems that this change fixed WinCairo flaky indexeddb test failures. > Thank you! Nice :)
Note You need to log in before you can comment on or make changes to this bug.