Bug 209499

Summary: REGRESSION: ASSERTION FAILED: m_wrapper on storage/indexeddb/modern/abort-requests tests
Product: WebKit Reporter: Jason Lawrence <Lawrence.j>
Component: WebCore JavaScriptAssignee: Chris Dumez <cdumez>
Status: RESOLVED FIXED    
Severity: Normal CC: aboya, achristensen, alecflett, beidson, cdumez, ews-watchlist, ggaren, jsbell, rniwa, sihui_liu, webkit-bot-watchers-bugzilla, webkit-bug-importer, youennf, ysuzuki
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Mac   
OS: macOS 10.14   
Attachments:
Description Flags
abort-requests-cancelled-private-crash-log
none
Patch none

Description Jason Lawrence 2020-03-24 14:31:56 PDT 
Created attachment 394414 [details]
abort-requests-cancelled-private-crash-log

storage/indexeddb/modern/abort-requests-cancelled.html
storage/indexeddb/modern/abort-requests-cancelled-private.html

Description:
These tests are flaky crashing on Catalina wk1. The crashes appear in the visible history on 03/12/2020.

History:
https://results.webkit.org/?flavor=wk1&version_name=Catalina&style=debug&suite=layout-tests&suite=layout-tests&test=storage%2Findexeddb%2Fmodern%2Fblocked-open-db-requests-private.html&test=storage%2Findexeddb%2Fodd-strings-private.html

Crash logs attached;
Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   com.apple.JavaScriptCore      	0x000000010a15687e WTFCrash + 14 (Assertions.cpp:309)
1   com.apple.WebCore             	0x000000012319e64b WTFCrashWithInfo(int, char const*, char const*, int) + 27
2   com.apple.WebCore             	0x00000001254d3cb8 WebCore::JSEventListener::jsFunction(WebCore::ScriptExecutionContext&) const + 616 (JSEventListener.h:123)
3   com.apple.WebCore             	0x00000001254d2fdb WebCore::JSEventListener::handleEvent(WebCore::ScriptExecutionContext&, WebCore::Event&) + 219 (JSEventListener.cpp:115)
4   com.apple.WebCore             	0x0000000125b8e5cc WebCore::EventTarget::innerInvokeEventListeners(WebCore::Event&, WTF::Vector<WTF::RefPtr<WebCore::RegisteredEventListener, WTF::DumbPtrTraits<WebCore::RegisteredEventListener> >, 1ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc>, WebCore::EventTarget::EventInvokePhase) + 956 (EventTarget.cpp:320)
5   com.apple.WebCore             	0x0000000125b8a802 WebCore::EventTarget::fireEventListeners(WebCore::Event&, WebCore::EventTarget::EventInvokePhase) + 354 (EventTarget.cpp:257)
6   com.apple.WebCore             	0x0000000125b75ef4 WebCore::EventContext::handleLocalEvents(WebCore::Event&, WebCore::EventTarget::EventInvokePhase) const + 228 (EventContext.cpp:58)
7   com.apple.WebCore             	0x0000000125b769eb WebCore::dispatchEventInDOM(WebCore::Event&, WebCore::EventPath const&) + 379 (EventDispatcher.cpp:101)
8   com.apple.WebCore             	0x0000000125b76df2 void WebCore::dispatchEventWithType<WebCore::EventTarget>(WTF::Vector<WebCore::EventTarget*, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc> const&, WebCore::Event&) + 338 (EventDispatcher.cpp:186)
Comment 1 Radar WebKit Bug Importer 2020-03-24 14:32:20 PDT 
<rdar://problem/60842165>
Comment 2 Jason Lawrence 2020-03-24 14:40:11 PDT 
I have marked these tests as crashing while this issue is investigated.
https://trac.webkit.org/changeset/258939/webkit
Comment 4 Chris Dumez 2020-03-26 12:35:26 PDT 
Created attachment 394641 [details]
Patch
Comment 5 EWS 2020-03-26 13:25:20 PDT 
Committed r259068: <https://trac.webkit.org/changeset/259068>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 394641 [details].
Comment 6 Sihui Liu 2020-03-27 15:38:12 PDT 
Wonder whether or not we need this change for IDBDatabase and IDBDatabaseRequest, which are also not consulting ActiveDOMObject::hasPendingActivity()
Comment 7 Sihui Liu 2020-03-27 15:39:41 PDT 
(In reply to Sihui Liu from comment #6)
> Wonder whether or not we need this change for IDBDatabase and
> IDBDatabaseRequest, which are also not consulting
> ActiveDOMObject::hasPendingActivity()

*IDBRequest*
Comment 8 Geoffrey Garen 2020-03-30 10:39:36 PDT 
(In reply to Sihui Liu from comment #6)
> Wonder whether or not we need this change for IDBDatabase and
> IDBDatabaseRequest, which are also not consulting
> ActiveDOMObject::hasPendingActivity()

Yes, I believe we do. (Both should call through to IDBActiveDOMObject.)
Comment 9 Chris Dumez 2020-03-30 10:41:28 PDT 
(In reply to Geoffrey Garen from comment #8)
> (In reply to Sihui Liu from comment #6)
> > Wonder whether or not we need this change for IDBDatabase and
> > IDBDatabaseRequest, which are also not consulting
> > ActiveDOMObject::hasPendingActivity()
> 
> Yes, I believe we do. (Both should call through to IDBActiveDOMObject.)

Every ActiveDOMObject::hasPendingActivity() override *should* check the base ActiveDOMObject::hasPendingActivity(). It may or may have a behavior change, depending on whether or not makePendingActivity() is used. Either way though, the safe thing to do is to always query the base ActiveDOMObject::hasPendingActivity().
Comment 10 Chris Dumez 2020-03-30 10:42:03 PDT 
(In reply to Chris Dumez from comment #9)
> (In reply to Geoffrey Garen from comment #8)
> > (In reply to Sihui Liu from comment #6)
> > > Wonder whether or not we need this change for IDBDatabase and
> > > IDBDatabaseRequest, which are also not consulting
> > > ActiveDOMObject::hasPendingActivity()
> > 
> > Yes, I believe we do. (Both should call through to IDBActiveDOMObject.)
> 
> Every ActiveDOMObject::hasPendingActivity() override *should* check the base
> ActiveDOMObject::hasPendingActivity(). It may or may have a behavior change,
> depending on whether or not makePendingActivity() is used. Either way
> though, the safe thing to do is to always query the base
> ActiveDOMObject::hasPendingActivity().

I think I will check ALL ActiveDOMObject::hasPendingActivity() overrides and fix them all up at once.
Comment 11 Chris Dumez 2020-03-30 13:13:12 PDT 
*** Bug 209744 has been marked as a duplicate of this bug. ***