1530284 - Database is destructed without calling CleanupMetadata()

Status: RESOLVED DUPLICATE of bug 1539538

(Core :: Storage: IndexedDB, enhancement, P2)

Description

[Tom Tung [:tt, :ttung]]

I believe this is one causes leads shutdown to hang.

Because having a method to just iterate the mLiveDatabases array in the gLiveDatabasehashtable might not be a good idea, I'm working on finding the place which causes the issue if it's possible.

While reading the code, I'm a bit suspect the code in [1]. So, the helper is held by the Database, the helper holds the callback, the Database will be removed in the CleanupMetadata later, and they all live in the Background thread. However, if the IPC is about to close the channel, there seems to be a chance to kill the Database object in the Background thread. In this case, the callback is not executed, all the stuff is destructed, and thus the Database is not removed from the table in the gLiveDatabaseHashtable.

I have a patch for testing to try to catch the Database, but if I cannot hit the problem, I'm thinking to provide a patch to fix this directly.

[1] https://searchfox.org/mozilla-central/source/dom/indexedDB/ActorsParent.cpp#12655-12661
[2] https://searchfox.org/mozilla-central/rev/dc0adc07db3df9431a0876156f50c65d580010cb/dom/indexedDB/ActorsParent.cpp#12230-12237

The procedure of the deletion of IPC subtree.
https://searchfox.org/mozilla-central/source/__GENERATED__/ipc/ipdl/PBackgroundIDBDatabaseParent.cpp#143-146

Comment 1

[Jan Varga [:janv]]

If IPC kills the actor it should call ActorDestroy and we have this code path there:
Database::ActorDestroy calls Database::Invalidate which calls CloseInternal and that calls MaybeCloseConnection in the end.

Comment 2

[Tom Tung [:tt, :ttung]]

(In reply to Jan Varga [:janv] from comment #1)

If IPC kills the actor it should call ActorDestroy and we have this code path there:
Database::ActorDestroy calls Database::Invalidate which calls CloseInternal and that calls MaybeCloseConnection in the end.

Thanks for the correction! That's true, then what I suspected is not correct.

Comment 3

[Tom Tung [:tt, :ttung]]

I'm testing a patch which checking the gLiveDatabaseHashtable on Database's dtor. If I fail to catch the issue locally, I will land this patch for diagnosis purpose based on our team meeting.

Comment 4

[Tom Tung [:tt, :ttung]]

Attachment: Bug 1530284 - Testing patch to catch the issue that deleting a Database object without removing it from the global hash table;

Comment 5

[Tom Tung [:tt, :ttung]]

Andrew told me that I could apply this to a certain amount of users on Nightly by some prefs instead of to them all if I really want to.

And, he also advised me that I could to do some to make users escape from the crash on the second time. I think it's doable by turning on the pref right before the crash and check the pref on each time before iterating the hashtable on destructor of the database object.

Comment 6

[Tom Tung [:tt, :ttung]]

It seems that this might be fixed by Jan in bug 1538619. And, the verification and a pointer check might be covered in bug 1539538. So, close this bug for now.