| Summary: | Crash in WebCore::SQLiteStatement::prepare() from ServiceWorker I/O Thread | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Daniel Bates <dbates> | ||||
| Component: | Service Workers | Assignee: | Brady Eidson <beidson> | ||||
| Status: | NEW --- | ||||||
| Severity: | Normal | CC: | beidson, cdumez, youennf | ||||
| Priority: | P2 | ||||||
| Version: | WebKit Local Build | ||||||
| Hardware: | Unspecified | ||||||
| OS: | Unspecified | ||||||
| See Also: |
https://bugs.webkit.org/show_bug.cgi?id=118563 https://bugs.webkit.org/show_bug.cgi?id=180589 |
||||||
| Attachments: |
|
||||||
|
Description
Daniel Bates
2017-12-06 19:43:46 PST
I also got the same crash in EWS bot in processing results of https://bugs.webkit.org/show_bug.cgi?id=179641 What makes sure m_database is destroyed on the background thread? I see this assertion ASSERT(!m_database); in the ~RegistrationDatabase() destructor (which gets called on the main thread) but I do not see the code that takes care of clearing m_database on the BG thread. (In reply to Chris Dumez from comment #2) > What makes sure m_database is destroyed on the background thread? I see this > assertion ASSERT(!m_database); in the ~RegistrationDatabase() destructor > (which gets called on the main thread) but I do not see the code that takes > care of clearing m_database on the BG thread. The RegistrationDatabase is never destroyed. (In reply to Brady Eidson from comment #3) > (In reply to Chris Dumez from comment #2) > > What makes sure m_database is destroyed on the background thread? I see this > > assertion ASSERT(!m_database); in the ~RegistrationDatabase() destructor > > (which gets called on the main thread) but I do not see the code that takes > > care of clearing m_database on the BG thread. > > The RegistrationDatabase is never destroyed. Elaborating: Are SWServers ever destroyed? If not, then RegistrationStores aren't, and RegistrationDatabases aren't. I actually think what's happening is a pushChanges() is happening without an m_database Indeed, there's no protection against database operations happening without a valid database. Adding that soon. (Would be great to know why the DB isn't being created) Created attachment 328833 [details]
Patch
Comment on attachment 328833 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=328833&action=review > Source/WebCore/workers/service/server/RegistrationDatabase.cpp:204 > + if (!m_successfullyOpened) { If you call pushChanges() right away after constructing the RegistrationDatabase, m_successfullyOpened may not have been set to true yet :/ (In reply to Chris Dumez from comment #9) > Comment on attachment 328833 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=328833&action=review > > > Source/WebCore/workers/service/server/RegistrationDatabase.cpp:204 > > + if (!m_successfullyOpened) { > > If you call pushChanges() right away after constructing the > RegistrationDatabase, m_successfullyOpened may not have been set to true yet > :/ Such a thing does not happen, though. (In reply to Brady Eidson from comment #10) > (In reply to Chris Dumez from comment #9) > > Comment on attachment 328833 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=328833&action=review > > > > > Source/WebCore/workers/service/server/RegistrationDatabase.cpp:204 > > > + if (!m_successfullyOpened) { > > > > If you call pushChanges() right away after constructing the > > RegistrationDatabase, m_successfullyOpened may not have been set to true yet > > :/ > > Such a thing does not happen, though. Correction: *should* not happen Comment on attachment 328833 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=328833&action=review > Source/WebCore/workers/service/server/RegistrationDatabase.cpp:205 > + LOG_ERROR("Attempt to push changes to SW registration database that is not open"); Should be RELEASE_LOG_ERROR. Going with the one in 180587 for now, will revisit later. (I really need to get 180573 resolved) Brady and I think the real underlying issue is https://bugs.webkit.org/show_bug.cgi?id=180589. |