1228937 - "Assertion failure: gFactoryOps->IsEmpty()" with lots of deleteDatabase

Status: RESOLVED FIXED

(Core :: Storage: IndexedDB, defect, P5)

Description

[Jesse Ruderman]

Attachment: testcase

Assertion failure: gFactoryOps->IsEmpty(), at dom/indexedDB/ActorsParent.cpp:12757

(Easier to reproduce with a debug+opt build from Tinderbox than with a local debug build?)

Comment 1

[Jesse Ruderman]

Attachment: stack

Comment 2

[Andrew Sutherland [:asuth] (he/him)]

As sorta covered on bug 1246828 comment 12, either:
- Factory::ActorDestroy should stop trying to (synchronously) clean up when it hits zero live factories.
- FactoryOp::ActorDestroy should synchronously clean itself up

This is necessary because Factory manages the FactoryOps and they're all destroyed synchronously with no turns of the event loop in-between.  Because the factory ops have the added complexity of their mDelayedOp mechanism, modifying factory may be the cleanest option.  While it could use a runnable that waits on the gFactoryOps to complete, its data structures seem relatively minimal and so it might make sense to move their destruction to the IDB quota client shutdown since that's a reasonable shutdown boundary.

Comment 3

[Jan Varga [:janv]]

Andrew, thanks for this analysis!

Comment 4

[Jan Varga [:janv]]

(In reply to Andrew Sutherland [:asuth] from comment #2)
> As sorta covered on bug 1246828 comment 12, either:
> - Factory::ActorDestroy should stop trying to (synchronously) clean up when
> it hits zero live factories.
> - FactoryOp::ActorDestroy should synchronously clean itself up
> 
> This is necessary because Factory manages the FactoryOps and they're all
> destroyed synchronously with no turns of the event loop in-between.  Because
> the factory ops have the added complexity of their mDelayedOp mechanism,
> modifying factory may be the cleanest option.  While it could use a runnable
> that waits on the gFactoryOps to complete, its data structures seem
> relatively minimal and so it might make sense to move their destruction to
> the IDB quota client shutdown since that's a reasonable shutdown boundary.

I fully agree, we had a synchronous cleanup in FactoryOp::ActoryDestroy(), but it was a huge pain in the ass, so we removed it. I'll try to move Factory::ActoryDestroy cleanup to the quota client shutdown.
Thanks again!.

Comment 5

[Jan Varga [:janv]]

This will be fixed by bug 1246828.

Comment 6

[Andrew Overholt [:overholt]]

Bug 1246828 is now fixed. Jesse, could you be so kind as to verify this is now fixed (Jan was telling me he had a hard time reproducing)?

Comment 7

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Assuming FIXED by bug 1246828 as per comment 5.