From 42a03f2556677e22ec3ec3de10c3e5d17127af4d Mon Sep 17 00:00:00 2001 From: Alex Date: Thu, 11 Sep 2025 14:02:03 -0500 Subject: [PATCH] fix: concurrency issue (#21830) --- .../services/background_worker.service.dart | 61 +++++++----- .../lib/domain/utils/sync_linked_album.dart | 5 +- mobile/lib/main.dart | 2 +- mobile/lib/utils/isolate.dart | 95 ++++++++++--------- 4 files changed, 90 insertions(+), 73 deletions(-) diff --git a/mobile/lib/domain/services/background_worker.service.dart b/mobile/lib/domain/services/background_worker.service.dart index 3bb0d7980f..4df221d41b 100644 --- a/mobile/lib/domain/services/background_worker.service.dart +++ b/mobile/lib/domain/services/background_worker.service.dart @@ -169,7 +169,10 @@ class BackgroundWorkerBgService extends BackgroundWorkerFlutterApi { } try { + final backgroundSyncManager = _ref.read(backgroundSyncProvider); _isCleanedUp = true; + _ref.dispose(); + _cancellationToken.cancel(); _logger.info("Cleaning up background worker"); final cleanupFutures = [ @@ -179,14 +182,13 @@ class BackgroundWorkerBgService extends BackgroundWorkerFlutterApi { }), _drift.close(), _driftLogger.close(), - _ref.read(backgroundSyncProvider).cancel(), - _ref.read(backgroundSyncProvider).cancelLocal(), + backgroundSyncManager.cancel(), + backgroundSyncManager.cancelLocal(), ]; if (_isar.isOpen) { cleanupFutures.add(_isar.close()); } - _ref.dispose(); await Future.wait(cleanupFutures); _logger.info("Background worker resources cleaned up"); } catch (error, stack) { @@ -195,35 +197,42 @@ class BackgroundWorkerBgService extends BackgroundWorkerFlutterApi { } Future _handleBackup() async { - if (!_isBackupEnabled || _isCleanedUp) { - _logger.info("[_handleBackup 1] Backup is disabled. Skipping backup routine"); - return; - } + await runZonedGuarded( + () async { + if (!_isBackupEnabled || _isCleanedUp) { + _logger.info("[_handleBackup 1] Backup is disabled. Skipping backup routine"); + return; + } - _logger.info("[_handleBackup 2] Enqueuing assets for backup from the background service"); + _logger.info("[_handleBackup 2] Enqueuing assets for backup from the background service"); - final currentUser = _ref.read(currentUserProvider); - if (currentUser == null) { - _logger.warning("[_handleBackup 3] No current user found. Skipping backup from background"); - return; - } + final currentUser = _ref.read(currentUserProvider); + if (currentUser == null) { + _logger.warning("[_handleBackup 3] No current user found. Skipping backup from background"); + return; + } - _logger.info("[_handleBackup 4] Resume backup from background"); - if (Platform.isIOS) { - return _ref.read(driftBackupProvider.notifier).handleBackupResume(currentUser.id); - } + _logger.info("[_handleBackup 4] Resume backup from background"); + if (Platform.isIOS) { + return _ref.read(driftBackupProvider.notifier).handleBackupResume(currentUser.id); + } - final canPing = await _ref.read(serverInfoServiceProvider).ping(); - if (!canPing) { - _logger.warning("[_handleBackup 5] Server is not reachable. Skipping backup from background"); - return; - } + final canPing = await _ref.read(serverInfoServiceProvider).ping(); + if (!canPing) { + _logger.warning("[_handleBackup 5] Server is not reachable. Skipping backup from background"); + return; + } - final networkCapabilities = await _ref.read(connectivityApiProvider).getCapabilities(); + final networkCapabilities = await _ref.read(connectivityApiProvider).getCapabilities(); - return _ref - .read(uploadServiceProvider) - .startBackupWithHttpClient(currentUser.id, networkCapabilities.hasWifi, _cancellationToken); + return _ref + .read(uploadServiceProvider) + .startBackupWithHttpClient(currentUser.id, networkCapabilities.hasWifi, _cancellationToken); + }, + (error, stack) { + debugPrint("Error in backup zone $error, $stack"); + }, + ); } Future _syncAssets({Duration? hashTimeout}) async { diff --git a/mobile/lib/domain/utils/sync_linked_album.dart b/mobile/lib/domain/utils/sync_linked_album.dart index 9df69799ae..d4094f74cc 100644 --- a/mobile/lib/domain/utils/sync_linked_album.dart +++ b/mobile/lib/domain/utils/sync_linked_album.dart @@ -1,9 +1,10 @@ import 'package:hooks_riverpod/hooks_riverpod.dart'; +import 'package:immich_mobile/domain/models/store.model.dart'; import 'package:immich_mobile/domain/services/sync_linked_album.service.dart'; -import 'package:immich_mobile/providers/user.provider.dart'; +import 'package:immich_mobile/entities/store.entity.dart'; Future syncLinkedAlbumsIsolated(ProviderContainer ref) { - final user = ref.read(currentUserProvider); + final user = Store.tryGet(StoreKey.currentUser); if (user == null) { return Future.value(); } diff --git a/mobile/lib/main.dart b/mobile/lib/main.dart index 7e3d6152c9..4f74c30e3b 100644 --- a/mobile/lib/main.dart +++ b/mobile/lib/main.dart @@ -46,7 +46,7 @@ void main() async { await Bootstrap.initDomain(isar, drift, logDb); await initApp(); // Warm-up isolate pool for worker manager - await workerManager.init(dynamicSpawning: false); + await workerManager.init(dynamicSpawning: true); await migrateDatabaseIfNeeded(isar, drift); HttpSSLOptions.apply(); diff --git a/mobile/lib/utils/isolate.dart b/mobile/lib/utils/isolate.dart index cca1498e0f..75e51d4360 100644 --- a/mobile/lib/utils/isolate.dart +++ b/mobile/lib/utils/isolate.dart @@ -31,55 +31,62 @@ Cancelable runInIsolateGentle({ } return workerManager.executeGentle((cancelledChecker) async { - BackgroundIsolateBinaryMessenger.ensureInitialized(token); - DartPluginRegistrant.ensureInitialized(); + await runZonedGuarded( + () async { + BackgroundIsolateBinaryMessenger.ensureInitialized(token); + DartPluginRegistrant.ensureInitialized(); - final (isar, drift, logDb) = await Bootstrap.initDB(); - await Bootstrap.initDomain(isar, drift, logDb, shouldBufferLogs: false); - final ref = ProviderContainer( - overrides: [ - // TODO: Remove once isar is removed - dbProvider.overrideWithValue(isar), - isarProvider.overrideWithValue(isar), - cancellationProvider.overrideWithValue(cancelledChecker), - driftProvider.overrideWith(driftOverride(drift)), - ], - ); + final (isar, drift, logDb) = await Bootstrap.initDB(); + await Bootstrap.initDomain(isar, drift, logDb, shouldBufferLogs: false); + final ref = ProviderContainer( + overrides: [ + // TODO: Remove once isar is removed + dbProvider.overrideWithValue(isar), + isarProvider.overrideWithValue(isar), + cancellationProvider.overrideWithValue(cancelledChecker), + driftProvider.overrideWith(driftOverride(drift)), + ], + ); - Logger log = Logger("IsolateLogger"); + Logger log = Logger("IsolateLogger"); - try { - HttpSSLOptions.apply(applyNative: false); - return await computation(ref); - } on CanceledError { - log.warning("Computation cancelled ${debugLabel == null ? '' : ' for $debugLabel'}"); - } catch (error, stack) { - log.severe("Error in runInIsolateGentle ${debugLabel == null ? '' : ' for $debugLabel'}", error, stack); - } finally { - try { - await LogService.I.dispose(); - await logDb.close(); - await ref.read(driftProvider).close(); - - // Close Isar safely try { - final isar = ref.read(isarProvider); - if (isar.isOpen) { - await isar.close(); - } - } catch (e) { - debugPrint("Error closing Isar: $e"); - } + HttpSSLOptions.apply(applyNative: false); + return await computation(ref); + } on CanceledError { + log.warning("Computation cancelled ${debugLabel == null ? '' : ' for $debugLabel'}"); + } catch (error, stack) { + log.severe("Error in runInIsolateGentle ${debugLabel == null ? '' : ' for $debugLabel'}", error, stack); + } finally { + try { + ref.dispose(); - ref.dispose(); - } catch (error, stack) { - debugPrint("Error closing resources in isolate: $error, $stack"); - } finally { - ref.dispose(); - // Delay to ensure all resources are released - await Future.delayed(const Duration(seconds: 2)); - } - } + await LogService.I.dispose(); + await logDb.close(); + await drift.close(); + + // Close Isar safely + try { + if (isar.isOpen) { + await isar.close(); + } + } catch (e) { + debugPrint("Error closing Isar: $e"); + } + } catch (error, stack) { + debugPrint("Error closing resources in isolate: $error, $stack"); + } finally { + ref.dispose(); + // Delay to ensure all resources are released + await Future.delayed(const Duration(seconds: 2)); + } + } + return null; + }, + (error, stack) { + debugPrint("Error in isolate zone: $error, $stack"); + }, + ); return null; }); }