Skip to content

Commit

Permalink
store: Call connection.close in PerAccountStore.dispose
Browse files Browse the repository at this point in the history
This seems hygienic. It makes sense to explicitly clean up HTTP
client resources when we know we'll be setting up a new client
instance to replace the old one. (The dead-queue reload is currently
the only path where PerAccountStore.dispose is called.)

It will also make sense to clean up these resources when an account
is logged out. That's a feature we'll be implementing soon, with
PerAccountStore.dispose in that new path as well.

The "TODO abort long-poll and close ApiConnection" was on
UpdateMachine's dispose method, not PerAccountStore's. But since
PerAccountStore is what owns the connection, its dispose method
seemed like the more appropriate place to close the connection.
  • Loading branch information
chrisbobbe committed Oct 17, 2024
1 parent 7aff4d1 commit f3a3361
Showing 1 changed file with 6 additions and 1 deletion.
7 changes: 6 additions & 1 deletion lib/model/store.dart
Original file line number Diff line number Diff line change
Expand Up @@ -464,6 +464,7 @@ class PerAccountStore extends ChangeNotifier with EmojiStore, ChannelStore, Mess
_messages.dispose();
typingStatus.dispose();
updateMachine?.dispose();
connection.close();
_disposed = true;
super.dispose();
}
Expand Down Expand Up @@ -980,7 +981,11 @@ class UpdateMachine {
///
/// After this is called, the instance is not in a usable state
/// and should be abandoned.
void dispose() { // TODO abort long-poll and close ApiConnection
///
/// To abort polling mid-request, [store]'s [PerAccountStore.connection]
/// needs to be closed using [ApiConnection.close], which causes in-progress
/// requests to error. [PerAccountStore.dispose] does that.
void dispose() {
assert(!_disposed);
NotificationService.instance.token.removeListener(_registerNotificationToken);
_disposed = true;
Expand Down

0 comments on commit f3a3361

Please sign in to comment.