drift
drift copied to clipboard
Potential Memory Leak when opening and closing connections & lazy database closure
A potential memory leak occurs when repeatedly opening and closing connections in Drift. Instances of DriftCommunication accumulate over time and are not properly deallocated, leading to increasing memory usage. This issue suggests that resources tied to DriftCommunication are not being released when a connection is closed. NotifyTablesUpdated and TableUpdate are also affected.
Closing a lazy database without performing any operations beforehand results in an increasing number of isolates, suggesting a resource leak as well.
Steps to Reproduce:
- Open a connection to a Drift database.
QueryExecutor openConnection(String dbPath) {
return driftDatabase(
name: dbPath,
native: DriftNativeOptions(
shareAcrossIsolates: false,
databasePath: () async {
return join(await getDatabasesPath(), dbPath);
},
),
);
}
- Close the connection.
- Repeat the open/close cycle multiple times.
- Monitor memory usage or track DriftCommunication instances.
Using:
- flutter: 3.24.5
- drift: 2.23.1
- drift_flutter: 0.2.4
Thanks for the report! I've tried to reproduce it, but unfortunately wasn't successful. I've used this program:
void main() async {
FlutterError.demangleStackTrace = (StackTrace stack) {
if (stack is Trace) return stack.vmTrace;
if (stack is Chain) return stack.toTrace().vmTrace;
return stack;
};
for (var i = 0; i < 10; i++) {
final db = AppDatabase(driftDatabase(
name: 'repro.db',
native: DriftNativeOptions(
shareAcrossIsolates: false,
databasePath: () async => '/tmp/test.db',
),
));
await db.customStatement('SELECT 1;');
await db.close();
}
print('at end');
}
Setting a breakpoint on the print line at the end, opening devtools and starting one manual GC only shows a single DriftCommunication object afterwards.
Thanks for the fast response! I appreciate your time in trying to reproduce the issue.
With your example, I also couldn't reproduce the issue. However, I forgot to mention an important detail...I am using Drift in combination with workmanager (v.0.5.2), where the database is opened and closed within a background task.
It’s possible that the issue only occurs in this specific setup. Could you try testing it in a workmanager background task to see if the behavior changes?
For the second issue (lazy database closure causing isolate leaks), I was able to reproduce it with your program if I remove the following line:
await db.customStatement('SELECT 1;');
Without this statement, the number of isolates increases each time the database is opened and closed.
Good point, thanks! I've fixed that in 49ebf8828755283d43bcd00906c437a0c11594dd. I'm not convinced that issue is what you were seeing with workmanager since I assume you probably used the database there. To help me reproduce that, could you share how you're closing the database in the workmanager task? If the main database class isn't garbage collected (e.g. due to an unrelated leak, it's also possible for the other instance to be tied to that). Is the database opened within the task and not used anywhere else?
Oh well, as you suggested, I forgot to close the database, so it remained open and unused. After properly closing the database in the WorkManager task, the memory leak is now fixed.
Thanks for pointing it out!
We also had an aggregation of DriftCommunication instances. While this problem was fixed by calling db.close() as described, we still have an aggregation of NotifyTableUpdated and TableUpdate instances:
@devroble As I can see in your debugger-screenshot you also had an aggregation of these instances. Do you still have the same problem in your case?
I couldn't reproduce that problem in a small demo that listens to stream queries and then closes an isolate database repeatedly. Are you listening to table updates manually or is this just through stream queries?
We also had an aggregation of
DriftCommunicationinstances. While this problem was fixed by callingdb.close()as described, we still have an aggregation ofNotifyTableUpdatedandTableUpdateinstances:[@devroble](https://github.com/devroble) As I can see in your debugger-screenshot you also had an aggregation of these instances. Do you still have the same problem in your case?
Good observation, @johannessachse ! Yes, the aggregation of instances still exists, so there does seem to be an issue. While closing the database in the WorkManager task fixed one part of the problem, the accumulation of NotifyTableUpdated and TableUpdate instances is still happening. Are you setting shareAcrossIsolates: true as well?
I assume, that this then()-part in server_impl.dart seems not to be triggered in my case:
/// Creates a server from the underlying connection and further options.
ServerImplementation(this.connection, this.allowRemoteShutdown,
this.closeExecutorWhenShutdown) {
done.then((_) {
_closeRemainingConnections();
_tableUpdateNotifications.close();
});
}
Retaining path for NotifyTableUpdated:
dart:async/_DelayedData
dart:async/_PendingEvents
dart:async/_AsyncStreamController
package:drift/src/remote/server_impl.dart/ServerImplementation
package:drift/src/isolate.dart/RunningDriftServer
Closure Context
dart:core/_Closure
dart:async/_ControllerSubscription
dart:async/_SyncStreamController
dart:isolate/_RawReceivePort
dart:core/_List
dart:_compact_hash/_Map
Isolate
Retaining path for TableUpdate:
dart:core/_List
dart:core/_GrowableList
package:drift/src/remote/protocol.dart/NotifyTablesUpdated
dart:async/_DelayedData
dart:async/_PendingEvents
dart:async/_AsyncStreamController
package:drift/src/remote/server_impl.dart/ServerImplementation
package:drift/src/isolate.dart/RunningDriftServer
Closure Context
dart:core/_Closure
dart:async/_ControllerSubscription
dart:async/_SyncStreamController
dart:isolate/_RawReceivePort
dart:_compact_hash/_Map
Isolate
Hi, I work with Johannes and since he is on a well deserved vacation, I will try to pick this up.
@devroble We are not using the drift_flutter & shareAcrossIsolates since our setup is older. We manually share the port.
But I think our setup pretty much reflects the same usage.
The retain paths seem to indicate that this is being retained on the (drift) server side (drift isolate)., right?
@simolus3 Not sure how to reproduce this in a small sample.
Should we create another issue or reopen this issue?
For the second issue (lazy database closure causing isolate leaks), I was able to reproduce it with your program if I remove the following line:
await db.customStatement('SELECT 1;');Without this statement, the number of isolates increases each time the database is opened and closed.
I'm sure if this is the same issue, but I get multiple instances of the "Drift isolate worker for /path/to/database.sqlite".
Steps to reproduct:
- On Android enable "Don't keep activities" in Developer options
- Run the example app in
example/app(I did it from VSCode) - Sent the app to the background and resume.
- You should now have a second "Drift isolate worker"
- Sent the app to the background again and resume.
- You should now have a third "Drift isolate worker"
Thanks for that report, that's interesting to know! I don't think it's related to this issue, but it's something we can fix in drift anyway.
[@devroble](https://github.com/devroble) As I can see in your debugger-screenshot you also had an aggregation of these instances. Do you still have the same problem in your case?