Closed Bug 1150023 Opened 5 years ago Closed 5 years ago

Intermittent browser_940107_home_button_in_bookmarks_toolbar.js | application crashed [@ mozilla::dom::quota::QuotaManager::AbortCloseStoragesForProcess(mozilla::dom::ContentParent *)][@ mozilla::ipc::MessageChannel::NotifyChannelClosed()]

Categories

(Core :: DOM: IndexedDB, defect)

x86
Windows 8
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla40
blocking-b2g 2.2+
Tracking Status
firefox38 --- unaffected
firefox39 --- unaffected
firefox40 --- fixed
firefox-esr31 --- unaffected
b2g-v2.2 --- fixed
b2g-master --- fixed

People

(Reporter: cbook, Assigned: janv)

References

()

Details

(Keywords: crash, intermittent-failure)

Attachments

(2 files)

Windows 8 64-bit mozilla-inbound opt test mochitest-e10s-browser-chrome-1

https://treeherder.mozilla.org/logviewer.html#?job_id=8352679&repo=mozilla-inbound

07:09:51 WARNING - PROCESS-CRASH | browser/components/customizableui/test/browser_940107_home_button_in_bookmarks_toolbar.js | application crashed [@ mozilla::dom::quota::QuotaManager::AbortCloseStoragesForProcess(mozilla::dom::ContentParent *)]
07:09:51 INFO - Crash dump filename: c:\users\cltbld~1.t-w\appdata\local\temp\tmpdoo8dx.mozrunner\minidumps\64782377-b916-41a4-aa25-04b6e138d886.dmp
07:09:51 INFO - Operating system: Windows NT
07:09:51 INFO - 6.2.9200
07:09:51 INFO - CPU: amd64
07:09:51 INFO - family 6 model 30 stepping 5
07:09:51 INFO - 8 CPUs
07:09:51 INFO - Crash reason: EXCEPTION_ACCESS_VIOLATION_READ
07:09:51 INFO - Crash address: 0xe0000000d
07:09:51 INFO - Thread 0 (crashed)
07:09:51 INFO - 0 xul.dll!mozilla::dom::quota::QuotaManager::AbortCloseStoragesForProcess(mozilla::dom::ContentParent *) [QuotaManager.cpp:72d01e62e037 : 1736 + 0x10]
07:09:51 INFO - rbx = 0x0000001fa1de3400 r12 = 0x0000000000000000
07:09:51 INFO - r13 = 0x0000001f967ab000 r14 = 0x0000000000000003
07:09:51 INFO - r15 = 0x0000001f8539e6f8 rip = 0x000007ff34bfc17f
07:09:51 INFO - rsp = 0x0000001f8539e6d0 rbp = 0x0000001f8539e800
07:09:51 INFO - Found by: given as instruction pointer in context
07:09:51 INFO - 1 xul.dll!mozilla::dom::ContentParent::ShutDownProcess(mozilla::dom::ContentParent::ShutDownMethod) [ContentParent.cpp:72d01e62e037 : 1581 + 0xa]
07:09:51 INFO - rbx = 0x0000001fa1de3400 r12 = 0x0000000000000000
07:09:51 INFO - r13 = 0x0000001f967ab000 r14 = 0x0000000000000003
07:09:51 INFO - r15 = 0x0000001f8539e6f8 rip = 0x000007ff34d21584
07:09:51 INFO - rsp = 0x0000001f8539e760 rbp = 0x0000001f8539e800
07:09:51 INFO - Found by: call frame info
07:09:51 INFO - 2 xul.dll!mozilla::dom::ContentParent::RecvFinishShutdown() [ContentParent.cpp:72d01e62e037 : 1636 + 0x9]
07:09:51 INFO - rbx = 0x0000001fa1de3400 r12 = 0x0000000000000000
07:09:51 INFO - r13 = 0x0000001f967ab000 r14 = 0x0000000000000003
07:09:51 INFO - r15 = 0x0000001f8539e6f8 rip = 0x000007ff34d1cdda
07:09:51 INFO - rsp = 0x0000001f8539e790 rbp = 0x0000001f8539e800
07:09:51 INFO - Found by: call frame info
07:09:51 INFO - 3 xul.dll!mozilla::dom::PContentParent::OnMessageReceived(IPC::Message const &) [PContentParent.cpp:72d01e62e037 : 4872 + 0xb]
07:09:51 INFO - rbx = 0x0000001fa1de3400 r12 = 0x0000000000000000
07:09:51 INFO - r13 = 0x0000001f967ab000 r14 = 0x0000000000000003
07:09:51 INFO - r15 = 0x0000001f8539e6f8 rip = 0x000007ff34030780
07:09:51 INFO - rsp = 0x0000001f8539e7c0 rbp = 0x0000001f8539e800
07:09:51 INFO - Found by: call frame info
07:09:51 INFO - 4 xul.dll!mozilla::ipc::MessageChannel::DispatchAsyncMessage(IPC::Message const &) [MessageChannel.cpp:72d01e62e037 : 1224 + 0xd]
07:09:51 INFO - rbx = 0x0000001fa1de3400 r12 = 0x0000000000000000
07:09:51 INFO - r13 = 0x0000001f967ab000 r14 = 0x0000000000000003
07:09:51 INFO - r15 = 0x0000001f8539e6f8 rip = 0x000007ff33efa9de
07:09:51 INFO - rsp = 0x0000001f8539ecc0 rbp = 0x0000001f8539e800
07:09:51 INFO - Found by: call frame info
07:09:51 INFO - 5 xul.dll!mozilla::ipc::MessageChannel::DispatchMessageW(IPC::Message const &) [MessageChannel.cpp:72d01e62e037 : 1151 + 0x4]
07:09:51 INFO - rbx = 0x0000001fa1de3400 r12 = 0x0000000000000000
07:09:51 INFO - r13 = 0x0000001f967ab000 r14 = 0x0000000000000003
07:09:51 INFO - r15 = 0x0000001f8539e6f8 rip = 0x000007ff33efadde
07:09:51 INFO - rsp = 0x0000001f8539ed00 rbp = 0x0000001f8539e800
07:09:51 INFO - Found by: call frame info
07:09:51 INFO - 6 xul.dll!mozilla::ipc::MessageChannel::OnMaybeDequeueOne() [MessageChannel.cpp:72d01e62e037 : 1135 + 0xc]
07:09:51 INFO - rbx = 0x0000001fa1de3400 r12 = 0x0000000000000000
Component: DOM → DOM: IndexedDB
Flags: needinfo?(bent.mozilla)
Summary: Intermittent browser_940107_home_button_in_bookmarks_toolbar.js | application crashed [@ mozilla::dom::quota::QuotaManager::AbortCloseStoragesForProcess(mozilla::dom::ContentParent *)] → Intermittent browser_940107_home_button_in_bookmarks_toolbar.js | application crashed [@ mozilla::dom::quota::QuotaManager::AbortCloseStoragesForProcess(mozilla::dom::ContentParent *)][@ mozilla::ipc::MessageChannel::NotifyChannelClosed()]
I pushed a patch with debug printfs to try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=e22816da2548

Will see if it helps...
(In reply to Jan Varga [:janv] from comment #20)
> I pushed a patch with debug printfs to try

Thanks!
Flags: needinfo?(bent.mozilla)
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/Jan.Varga@gmail.com-e22816da2548/try-win64/try_win8_64_test-mochitest-e10s-browser-chrome-1-bm109-tests1-windows-build343.txt.gz

It seems that UnregisterStorage is not called before DatabaseOfflineStorage destructor.
I'm not entirely sure since I forgot to printf |this| pointer.
I pushed a test patch and we're deleting the object with this stack:

13:37:13 INFO - 0 xul.dll!mozilla::dom::indexedDB::`anonymous namespace'::DatabaseOfflineStorage::~DatabaseOfflineStorage() [ActorsParent.cpp:b5c2724aba76 : 7898 + 0x60]
13:37:13 INFO - 1 xul.dll!mozilla::dom::indexedDB::`anonymous namespace'::DatabaseOfflineStorage::Release() [ActorsParent.cpp:b5c2724aba76 : 15240 + 0x2a]
13:37:13 INFO - 2 xul.dll!mozilla::dom::indexedDB::`anonymous namespace'::Database::~Database() [ActorsParent.cpp:b5c2724aba76 : 5648 + 0x8c]
13:37:13 INFO - 3 xul.dll!mozilla::dom::indexedDB::`anonymous namespace'::TransactionBase::~TransactionBase() [ActorsParent.cpp:b5c2724aba76 : 11899 + 0x65]
13:37:13 INFO - 4 xul.dll!mozilla::dom::indexedDB::`anonymous namespace'::VersionChangeTransaction::`scalar deleting destructor'(unsigned int) + 0x13
13:37:13 INFO - 5 xul.dll!mozilla::dom::indexedDB::`anonymous namespace'::TransactionBase::CommitOp::TransactionFinishedAfterUnblock() [ActorsParent.cpp:b5c2724aba76 : 19207 + 0x1e]
13:37:13 INFO - 6 xul.dll!mozilla::dom::indexedDB::`anonymous namespace'::ConnectionPool::FinishCallbackWrapper::Run() [ActorsParent.cpp:b5c2724aba76 : 10509 + 0x8]
13:37:13 INFO - 7 xul.dll!nsThread::ProcessNextEvent(bool,bool *) [nsThread.cpp:b5c2724aba76 : 842 + 0x8]
13:37:13 INFO - 8 xul.dll!NS_ProcessNextEvent(nsIThread *,bool) [nsThreadUtils.cpp:b5c2724aba76 : 265 + 0xc]
xul.dll!mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate *) [MessagePump.cpp:b5c2724aba76 : 368 + 0xa]
Duplicate of this bug: 1152780
Ok, this debug patch makes it obvious what's the code path:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=17f33fdd4651

I'll try to come with a proper fix this weekend.
Attached patch patchSplinter Review
Ben, I think this is the right fix, I found out that sometimes even Factory::ActorDestroy() is called so I can't call database->Invalidate() in OpenDatabaseOp->SendResults() because gLiveDatabaseHashtable was already destroyed.

If this was a debug build then we would see many assertions like:
|MOZ_ASSERT(!gLiveDatabaseHashtable->Count());| in Factory::ActorDestroy() or
|MOZ_ASSERT(mClosed);| in Database::~Database

So we have to invalidate database objects referenced by OpenDatabaseOp if there where no actors for it and the last chance to do it is in OpenDatabaseOp::ActorDestroy() I think.
Assignee: nobody → Jan.Varga
Status: NEW → ASSIGNED
Attachment #8591232 - Flags: review?(bent.mozilla)
Comment on attachment 8591232 [details] [diff] [review]
patch

Review of attachment 8591232 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks ever so much for looking at this. One question before I stamp:

::: dom/indexedDB/ActorsParent.cpp
@@ +6587,5 @@
>  
>    virtual void
>    SendResults() override;
> +
> +  // IPDL methods.

Nit: Don't need this comment.

@@ +17606,5 @@
>      mVersionChangeTransaction = nullptr;
>    }
>  
> +  // Make sure to release the database on this thread.
> +  nsRefPtr<Database> database;

Hm, can you avoid the changes in this method if you change OpenDatabaseOp::ActorDestroy to only invalidate if |aWhy != Deletion| ?
(In reply to Ben Turner [:bent] (use the needinfo flag!) from comment #125)
> Comment on attachment 8591232 [details] [diff] [review]
> patch
> 
> Review of attachment 8591232 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Thanks ever so much for looking at this. One question before I stamp:

Sure, no problem :)

> 
> ::: dom/indexedDB/ActorsParent.cpp
> @@ +6587,5 @@
> >  
> >    virtual void
> >    SendResults() override;
> > +
> > +  // IPDL methods.
> 
> Nit: Don't need this comment.
> 
> @@ +17606,5 @@
> >      mVersionChangeTransaction = nullptr;
> >    }
> >  
> > +  // Make sure to release the database on this thread.
> > +  nsRefPtr<Database> database;
> 
> Hm, can you avoid the changes in this method if you change
> OpenDatabaseOp::ActorDestroy to only invalidate if |aWhy != Deletion| ?

Yeah, I actually considered to do it that way. Let me test it a bit ...
(In reply to Ben Turner [:bent] (use the needinfo flag!) from comment #125)
> Hm, can you avoid the changes in this method if you change
> OpenDatabaseOp::ActorDestroy to only invalidate if |aWhy != Deletion| ?

pushed to try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=9dd4e246488c
Comment on attachment 8591232 [details] [diff] [review]
patch

Review of attachment 8591232 [details] [diff] [review]:
-----------------------------------------------------------------

Try changes with that change look good, r=me!
Attachment #8591232 - Flags: review?(bent.mozilla) → review+
(In reply to Treeherder Robot from comment #130)
> log:
> https://treeherder.mozilla.org/logviewer.html#?repo=mozilla-
> inbound&job_id=8738744
> repository: mozilla-inbound
> start_time: 2015-04-10T21:24:44
> who: tomcat[at]mozilla[dot]com
> machine: t-w864-ix-010
> buildname: Windows 8 64-bit mozilla-inbound opt test
> mochitest-e10s-browser-chrome-1
> revision: a4dd555969f3
> 
> TEST-UNEXPECTED-FAIL |
> browser/components/customizableui/test/
> browser_940107_home_button_in_bookmarks_toolbar.js | application terminated
> with exit code 1
> PROCESS-CRASH |
> browser/components/customizableui/test/
> browser_940107_home_button_in_bookmarks_toolbar.js | application crashed [@
> mozilla::dom::quota::QuotaManager::AbortCloseStoragesForProcess(mozilla::dom:
> :ContentParent *)]
> Return code: 1

It seems someone starred an older build which didn't contain the fix.
https://hg.mozilla.org/mozilla-central/rev/047a8e82579f
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
[Approval Request Comment]
Bug caused by (feature/regressing bug #): IndexedDB on PBackground
User impact if declined: This backport is needed for bug 1152407, see comment 18 in that bug
Testing completed: The patch for m-c landed on 2015-04-13 
Risk to taking this patch (and alternatives if risky): Simple patch, easy to back up
String or UUID changes made by this patch: None
Attachment #8594965 - Flags: approval-mozilla-b2g37?
Attachment #8594965 - Flags: approval-mozilla-b2g37? → approval-mozilla-b2g37+
blocking-b2g: --- → 2.2+
You need to log in before you can comment on or make changes to this bug.