Bugzilla

(Arising from Bug 1710768, comment 4)

`nsMsgDatabase` maintains a list of message enumerators it has given out, and when the database is killed, it tells all the enumerators to invalidate themselves.
It does this upon nsMsgDatabase destruction, and in ForceClosed().
But it doesn't currently do it in nsMsgDatabase::Close(), and it _really_ seems like it should - it doesn't seem right to continue iterating over a closed database.

It's an easy fix... but then one of the gloda unit tests starts failing.

STEPS TO REPRODUCE:

Apply the fix:
```
+++ b/mailnews/db/msgdb/src/nsMsgDatabase.cpp
@@ -1425,4 +1425,5 @@ NS_IMETHODIMP nsMsgDatabase::Commit(nsMs
 
 NS_IMETHODIMP nsMsgDatabase::Close(bool forceCommit /* = TRUE */) {
+  InvalidateEnumerators();
   return CloseMDB(forceCommit);
 }
```

Run the [`test_index_messages_imap_offline.js`](https://searchfox.org/comm-central/source/mailnews/db/gloda/test/unit/test_index_messages_imap_offline.js) test:

```
./mach xpcshell-test comm/mailnews/db/gloda/test/unit/test_index_messages_imap_offline.js
```

Observed:

The `test_indexing_sweep` fails, in the `never before indexed folders` subtest.
It never gets past [`mailnews/db/gloda/test/unit/base_index_messages.js:143`](https://searchfox.org/comm-central/source/mailnews/db/gloda/test/unit/base_index_messages.js#143).

From peppering the C++ and JS with printf debugging, I can tell it's trying to index 5 messages. The first one seems OK, but then when it goes to index the second one, the nsMsgDatabase::Close() is called, invalidating the iterators. The indexing stops and the unit test fails.

Here's a dump of a run with the fix and my printf debugging, (annotated with #comments):
```
# the enumerator is created:
 0:02.90 pid:12216 XYZZY nsMsgDatabase[0x7f5603a93e40] GetFilterEnumerator => 0x7f5603c4bc80
# first message seems to index OK:
 0:02.90 pid:12216  foo3 0/5
 0:02.90 pid:12216 >>>  calling _indexMessage
 0:03.08 pid:12216 <<<  back from _indexMessage
# second message...
 0:03.09 pid:12216  foo3 1/5
 0:03.09 pid:12216 >>>  calling _indexMessage
# uh-oh... something is closing the database! bye bye enumerators...
 0:03.09 pid:12216 XYZZY nsMsgDatabase[0x7f5603a93e40] Close()
 0:03.09 pid:12216 XYZZY nsMsgDatabase[0x7f5603a93e40] invalidating 2/2 0x7f5603c4bc80
 0:03.09 pid:12216    XYZZY nsMsgDBEnumerator[0x7f5603c4bc80] Invalidate() (db=0x7f5603a93e40)
 0:03.09 pid:12216 XYZZY nsMsgDatabase[0x7f5603a93e40] invalidating 1/2 0x7f5603c4b4a0
 0:03.09 pid:12216    XYZZY nsMsgDBEnumerator[0x7f5603c4b4a0] Invalidate() (db=0x7f5603a93e40)
 0:03.24 pid:12216 <<<  back from _indexMessage
# the indexer tries to get the 3rd message, but the DB is now closed and the enumerator is now bogus:
 0:03.24 pid:12216 XYZZY nsMsgDBEnumerator[0x7f5603c4bc80] GetNext() on INVALID

# ...and the unit test fails because not all the expected messages were indexed.
```

And at this point I'm a little lost. I can't tell if gloda is actually doing something wrong, or if the unit test is doing something wrong...
The non-imap gloda tests seem to work OK, so perhaps theres some odd async racing happening where the test folder is removed before the indexing completes? I'm a little short on ideas now.

(Arising from Bug 1710768, comment 4)

`nsMsgDatabase` maintains a list of message enumerators it has given out, and when the database is killed, it tells all the enumerators to invalidate themselves.
It does this upon nsMsgDatabase destruction, and in ForceClosed().
But it doesn't currently do it in nsMsgDatabase::Close(), and it _really_ seems like it should - it doesn't seem right to continue iterating over a closed database.

It's an easy fix... but then one of the gloda unit tests starts failing.

STEPS TO REPRODUCE:

Apply the fix:
```
+++ b/mailnews/db/msgdb/src/nsMsgDatabase.cpp
@@ -1425,4 +1425,5 @@ NS_IMETHODIMP nsMsgDatabase::Commit(nsMs
 
 NS_IMETHODIMP nsMsgDatabase::Close(bool forceCommit /* = TRUE */) {
+  InvalidateEnumerators();
   return CloseMDB(forceCommit);
 }
```

Run the [`test_index_messages_imap_offline.js`](https://searchfox.org/comm-central/source/mailnews/db/gloda/test/unit/test_index_messages_imap_offline.js) test:

```
./mach xpcshell-test comm/mailnews/db/gloda/test/unit/test_index_messages_imap_offline.js
```

Observed:

The `test_indexing_sweep` fails, in the `never before indexed folders` subtest.
It never gets past [`mailnews/db/gloda/test/unit/base_index_messages.js:143`](https://searchfox.org/comm-central/source/mailnews/db/gloda/test/unit/base_index_messages.js#143).

From peppering the C++ and JS with printf debugging, I can tell it's trying to index 5 messages. The first one seems OK, but then when it goes to index the second one, the nsMsgDatabase::Close() is called, invalidating the iterators. The indexing stops and the unit test fails.

Here's a snippet of a run with the fix and my ugly printf debugging, annotated with #comments:
```
# the enumerator is created:
 0:02.90 pid:12216 XYZZY nsMsgDatabase[0x7f5603a93e40] GetFilterEnumerator => 0x7f5603c4bc80
# first message seems to index OK:
 0:02.90 pid:12216  foo3 0/5
 0:02.90 pid:12216 >>>  calling _indexMessage
 0:03.08 pid:12216 <<<  back from _indexMessage
# second message...
 0:03.09 pid:12216  foo3 1/5
 0:03.09 pid:12216 >>>  calling _indexMessage
# uh-oh... something is closing the database! bye bye enumerators...
 0:03.09 pid:12216 XYZZY nsMsgDatabase[0x7f5603a93e40] Close()
 0:03.09 pid:12216 XYZZY nsMsgDatabase[0x7f5603a93e40] invalidating 2/2 0x7f5603c4bc80
 0:03.09 pid:12216    XYZZY nsMsgDBEnumerator[0x7f5603c4bc80] Invalidate() (db=0x7f5603a93e40)
 0:03.09 pid:12216 XYZZY nsMsgDatabase[0x7f5603a93e40] invalidating 1/2 0x7f5603c4b4a0
 0:03.09 pid:12216    XYZZY nsMsgDBEnumerator[0x7f5603c4b4a0] Invalidate() (db=0x7f5603a93e40)
 0:03.24 pid:12216 <<<  back from _indexMessage
# the indexer tries to get the 3rd message, but the DB is now closed and the enumerator is now bogus:
 0:03.24 pid:12216 XYZZY nsMsgDBEnumerator[0x7f5603c4bc80] GetNext() on INVALID

# ...and the unit test fails because not all the expected messages were indexed.
```

And at this point I'm a little lost. I can't tell if gloda is actually doing something wrong, or if the unit test is doing something wrong...
The non-imap gloda tests seem to work OK, so perhaps theres some odd async racing happening where the test folder is removed before the indexing completes? I'm a little short on ideas now.