Example: https://tbpl.mozilla.org/php/getParsedLog.php?id=27113155&tree=Thunderbird-Trunk#error0 This looks like a mozilla-central change that broke comm-central. Bug 874814 looks to be the best candidate in the regression range.

Mmmh... We crash after 5 warnings saying that some stuff needs to be finalized. I'll investigate.

So, it seems that we have statements created by some JS code that are not finalized properly. Normally, this should have caused assertion failures. I believe that, for some reason, until now, they were caught by the garbage-collector and finalized properly at that moment. With the new behavior of closing, these statements are now auto-finalized when the connection is closed. Once garbage-collection catches up with these statements, it attempts to finalize them, which is not possible anymore because the connection is closed.

There are two errors involved here. 1/ comm-central code should finalize its statements - http://mxr.mozilla.org/comm-central/source/chat/components/src/imContacts.js#1213 (statement is never closed); - http://mxr.mozilla.org/comm-central/source/chat/components/src/imContacts.js#1206 (statement is never closed); - ... 2/ mozStorage should be more robust about preventing these situations.

Here's a quick (and untested) fix. I do not have time to test it today, though.

Try: https://tbpl.mozilla.org/?tree=Thunderbird-Try&rev=b585aebe2318

Comment on attachment 796659 [details] [diff] [review] Quick fix to comm-central Review of attachment 796659 [details] [diff] [review]: ----------------------------------------------------------------- ::: chat/components/src/imContacts.js @@ +143,5 @@ > let statement = DBConn.createStatement("SELECT id FROM tags where name = :name"); > statement.params.name = aName; > if (!statement.executeStep()) > return null; > + statement.finalize(); In another place with very similar code, you put the finalize statement in a try/finally block.

Try: https://tbpl.mozilla.org/?tree=Thunderbird-Try&rev=bc2bd194b7f7

executeAsyncThenFinalize can call finalize immediately after calling executeAsync. No need to wait for the execution to complete. So, it seems like the death problem is definitely that Statement::internalFinalize called by Statement::~Statement does: int srv = ::sqlite3_finalize(mDBStatement); mDBStatement = nullptr; regardless of whether the connection is alive or not. It needs to check the connection's closed state. (The connection must still be alive.) It seems like we can add a unit test to verify this becomes a non-crasher: - Create sync connection - Create sync statement, keep it alive - Run asyncClose on connection, wait for the close to notify as complete. - Finalize sync statement. Don't crash.

Try: https://tbpl.mozilla.org/?tree=Thunderbird-Try&rev=abeeb120fbd6

Comment on attachment 796918 [details] [diff] [review] Fixing statement management in imContacts.js I dislike the appearance of the code after this change, but I've nothing better to propose, so let's get this permaorange fixed. Yoric, thank you very much for looking into this so quickly! :-)

With all the oranges on Thunderbird-Trunk, let's hope that I'm not breaking something else without noticing.

Same one, with commit message.

Comment on attachment 797145 [details] [diff] [review] Fixing statement management in imContacts.js, v2 Review of attachment 797145 [details] [diff] [review]: ----------------------------------------------------------------- ::: chat/components/src/imContacts.js @@ +149,1 @@ > return this.getTagById(statement.row.id); This line no longer works as the statement has already been finalized when we reach it. @@ +1322,1 @@ > return BuddiesById[statement.row.id]; Same here. @@ +1344,2 @@ > buddy = > new Buddy(DBConn.lastInsertRowID, normalizedName, name, srvAlias, 0); This line no longer works because the 'name' and 'srvAlias' variables are now scoped to the try block.

http://hg.instantbird.org/instantbird/rev/f0cdff383d78 http://hg.instantbird.org/instantbird/rev/4f8f4112202f

Comment on attachment 797519 [details] [diff] [review] Follow up to fix issues in comment 24 https://hg.mozilla.org/comm-central/rev/a61f680923f2

We fixed another regression caused by this in https://bugzilla.instantbird.org/show_bug.cgi?id=2179