Fix minor memory leak after wallet is deleted
What changed, and why it matters
This commit fixes a minor memory leak that could occur after a wallet is deleted in the Skylight Wallet app. It adds safety checks so background timer tasks stop trying to work with a wallet that no longer exists, and it prevents an error in one stats-loading task from crashing the whole refresh process. The changes are defensive and improve stability, but they do not appear to be a security fix for an actively exploitable vulnerability.
Treat as a routine stability/maintenance patch. Review whether timer tasks are properly cancelled when a wallet is deleted, and confirm that swallowing loadAllStats exceptions does not mask sync or connectivity issues that should be surfaced to the user.
Security signals we found
Null-deref / use-after-free style guard added for deleted wallet object
Timer task lifecycle hardening after wallet deletion
Exception swallowing added to prevent refresh timer task failure propagation
No input validation, crypto, authentication, or privilege changes present
Evidence from the diff
The patch modifies lib/models/wallet_model.dart to guard three async methods (_runCheckConnectionTimerTask, _runRefreshTimerTask, and loadAllStats) against operating on a null _w2Wallet after wallet deletion. It adds warning logs and early returns when no wallet is open, and wraps loadAllStats().timeout() in a try/catch inside _runRefreshTimerTask so an exception does not propagate and leave timer state in an inconsistent or leaking condition. The commit title and message frame this as a ‘minor memory leak’ fix, not a security-relevant change.
Changed components
lib/models/wallet_model.dartWalletModel background timer tasks (_runCheckConnectionTimerTask, _runRefreshTimerTask)WalletModel.loadAllStats()Inspect captured patch +22 / −1
diff --git a/lib/models/wallet_model.dart b/lib/models/wallet_model.dart
index 62fcb60..096e087 100644
--- a/lib/models/wallet_model.dart
+++ b/lib/models/wallet_model.dart
@@ -176,6 +176,10 @@ class WalletModel with ChangeNotifier {
Future<void> _runCheckConnectionTimerTask() async {
if (_w2Wallet == null) {
+ log(
+ LogLevel.warn,
+ 'Attempted to run check connection timer task but there is no wallet open.',
+ );
return;
}
@@ -189,11 +193,20 @@ class WalletModel with ChangeNotifier {
Future<void> _runRefreshTimerTask() async {
if (_w2Wallet == null) {
+ log(
+ LogLevel.warn,
+ 'Attempted to run refresh timer task but there is no wallet open.',
+ );
return;
}
await refresh();
- await loadAllStats().timeout(Duration(seconds: 20));
+
+ try {
+ await loadAllStats().timeout(Duration(seconds: 20));
+ } catch (e) {
+ log(LogLevel.error, 'Error loading all stats: $e');
+ }
final txCount = _w2TxHistory!.count();
@@ -203,6 +216,14 @@ class WalletModel with ChangeNotifier {
}
Future<void> loadAllStats() async {
+ if (_w2Wallet == null) {
+ log(
+ LogLevel.warn,
+ 'Attempted to load all stats but there is no wallet open.',
+ );
+ return;
+ }
+
await Future.wait([
loadIsSynced(),
loadSyncedHeight(),
Why this scored 23/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.