Unhandled exception in dispose of message store

[gnprice]

I saw the following exception on my device just now:

[ERROR:flutter/runtime/dart_vm_initializer.cc(41)] Unhandled Exception: Concurrent modification during iteration: _Set len:0.
#0      _CompactIterator.moveNext (dart:collection-patch/compact_hash.dart:714)
#1      MessageStoreImpl.dispose (package:zulip/model/message.dart:57)
#2      PerAccountStore.dispose (package:zulip/model/store.dart:385)
#3      GlobalStore._setPerAccount (package:zulip/model/store.dart:135)
#4      GlobalStore._reloadPerAccount (package:zulip/model/store.dart:128)
<asynchronous suspension>
#5      UpdateMachine.poll (package:zulip/model/store.dart:733)
<asynchronous suspension>

The context was:

• I'd had the app open previously, and was looking at a message list
• Then I connected the device to my computer, and was looking at the logcat view in Android Studio
• Then I opened up the app again, from the background
• The error appeared a few seconds later.

From the stack trace, it looks like what happened is that the polling loop found the event queue had expired; it went to replace the store with a new one from a new initial fetch; and then it hit this exception while trying to dispose of the old store.

---

Looking at the call stack, I'm not sure there are user-visible symptoms from this error. It looks like there are only a few more steps that hadn't yet been taken, all of them various dispose calls.

On the other hand earlier this evening, I had observed a puzzling failure to replace the event queue:

• Slow sometimes to replace event queue after expiry #809

So if this could cause a symptom like that after all, then this could be the diagnosis for that issue #809. (That issue is why I was looking at logcat in the first place. But no luck; it'd been a couple of hours since the issue had happened, and the logs from that time were long since gone.)

In any case, we should fix the issue so that MessageStoreImpl.dispose completes successfully.

---

A PR fixing this issue should include two kinds of tests:

• A test focused on that dispose method: make a message store, register a message list or two on it, then call dispose. At a minimum this test can verify no exception is thrown; preferably it should also verify that dispose did its job.

• A direct regression test for this issue, which will be a bit more of an integration test: make a PerAccountStore and UpdateMachine; register a message list or two on the store; then induce an expired-queue exception in the polling loop so that it replaces the store, and expect no unhandled exceptions. (I believe the test framework will automatically fail the test if there's an unhandled exception.)

  For this test, take as a model the existing tests of the expired-queue recovery feature.

The new tests should fail when run on the code from before the PR (e.g. with git checkout origin -- lib).