Drop / reconnect database connections when app becomes backgrounded/active #467
Conversation
|
@nmalzieu you'll need a semantic commit to make this create a release. https://github.com/semantic-release/semantic-release |
ios/XMTPModule.swift
Outdated
| func reconnectAllLocalDatabaseConnections() async throws { | ||
| for (_, client) in clients { | ||
| // Call the method on each client | ||
| try await client.reconnectLocalDatabase() |
There was a problem hiding this comment.
This is a blocking call. Hopefully this won't create too much lag when people reopen the app if they have a lot of clients. I really think we should consider setting a limit of how many clients a user can be logged in with. For MVP I really think we should just allow 1.
There was a problem hiding this comment.
This is a blocking call but in an actor, isn't everything called in an async manner? I'm not used to actors
nmalzieu
force-pushed
the
noe/ios-db-disconnect
branch
from
3240f8c
to
65af5a9
Compare
| OnAppBecomesActive { | ||
| Task { | ||
| try await clientsManager.reconnectAllLocalDatabaseConnections() | ||
| } | ||
| } | ||
|
|
||
|
|
||
| OnAppEntersBackground { | ||
| Task { | ||
| try await clientsManager.dropAllLocalDatabaseConnections() | ||
| } | ||
| } |
There was a problem hiding this comment.
I think we need to be a little more carful about this since these methods will error for any client that does not pass a V3Enabled flag. So they should only be called if the client is v3enabled.
ios/XMTPModule.swift
Outdated
|
|
||
| OnAppBecomesActive { | ||
| Task { | ||
| try await clientsManager.reconnectAllLocalDatabaseConnections() | ||
| } | ||
| } |
There was a problem hiding this comment.
Is this a duplicate of line 1504
There was a problem hiding this comment.
sorry about that must have been the conflict when rebasing. Fixing.
|
🎉 This PR is included in version 2.4.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Since we have so many issues with db locks, I suspect we have an issue calling the db disconnect at the right moment.
My thinking is that there are issues either in the Converse code or the XMTP React Native code, that blocks the bridge for too long and we don't manage to actually dicsonnect db before the app is suspended
By adding that code directly in Swift we bypass the bridge and should be able to prevent some of these crashes
Possible improvements:
Clientinstance and can just call the dropDb method themselves!Also some weirdness: why is
client.reconnectLocalDatabaseasync butclient.dropLocalDatabaseConnectionsync?