Skip to content

Commit 88c0243

Browse files
alextran1502alextran1502
authored
chore: robust isolation tasks coordination (#21605)
* chore: robust isolation tasks coordination * give more time for database transaction to clean up * chore: clean up logs * chore: clean up logs * fix: logs
1 parent 3a29522 commit 88c0243

File tree

3 files changed

+196
-65
lines changed

3 files changed

+196
-65
lines changed

‎mobile/lib/domain/utils/background_sync.dart‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ class BackgroundSyncManager {
2424
Cancelable<void>? _syncTask;
2525
Cancelable<void>? _syncWebsocketTask;
2626
Cancelable<void>? _deviceAlbumSyncTask;
27+
Cancelable<void>? _linkedAlbumSyncTask;
2728
Cancelable<void>? _hashTask;
2829

2930
BackgroundSyncManager({
@@ -53,6 +54,12 @@ class BackgroundSyncManager {
5354
_syncWebsocketTask?.cancel();
5455
_syncWebsocketTask = null;
5556

57+
if (_linkedAlbumSyncTask != null) {
58+
futures.add(_linkedAlbumSyncTask!.future);
59+
}
60+
_linkedAlbumSyncTask?.cancel();
61+
_linkedAlbumSyncTask = null;
62+
5663
try {
5764
await Future.wait(futures);
5865
} on CanceledError {
@@ -158,8 +165,14 @@ class BackgroundSyncManager {
158165
}
159166

160167
Future<void> syncLinkedAlbum() {
161-
final task = runInIsolateGentle(computation: syncLinkedAlbumsIsolated);
162-
return task.future;
168+
if (_linkedAlbumSyncTask != null) {
169+
return _linkedAlbumSyncTask!.future;
170+
}
171+
172+
_linkedAlbumSyncTask = runInIsolateGentle(computation: syncLinkedAlbumsIsolated);
173+
return _linkedAlbumSyncTask!.whenComplete(() {
174+
_linkedAlbumSyncTask = null;
175+
});
163176
}
164177
}
165178

‎mobile/lib/providers/app_life_cycle.provider.dart‎

Lines changed: 180 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
import 'dart:async';
22

3-
import 'package:flutter/foundation.dart';
43
import 'package:hooks_riverpod/hooks_riverpod.dart';
54
import 'package:immich_mobile/domain/services/log.service.dart';
65
import 'package:immich_mobile/domain/utils/isolate_lock_manager.dart';
@@ -34,6 +33,12 @@ class AppLifeCycleNotifier extends StateNotifier<AppLifeCycleEnum> {
3433
final Ref _ref;
3534
bool _wasPaused = false;
3635

36+
// Add operation coordination
37+
Completer<void>? _resumeOperation;
38+
Completer<void>? _pauseOperation;
39+
40+
final _log = Logger("AppLifeCycleNotifier");
41+
3742
AppLifeCycleNotifier(this._ref) : super(AppLifeCycleEnum.active);
3843

3944
AppLifeCycleEnum getAppState() {
@@ -43,6 +48,32 @@ class AppLifeCycleNotifier extends StateNotifier<AppLifeCycleEnum> {
4348
void handleAppResume() async {
4449
state = AppLifeCycleEnum.resumed;
4550

51+
// Prevent overlapping resume operations
52+
if (_resumeOperation != null && !_resumeOperation!.isCompleted) {
53+
await _resumeOperation!.future;
54+
return;
55+
}
56+
57+
// Cancel any ongoing pause operation
58+
if (_pauseOperation != null && !_pauseOperation!.isCompleted) {
59+
_pauseOperation!.complete();
60+
}
61+
62+
_resumeOperation = Completer<void>();
63+
64+
try {
65+
await _performResume();
66+
} catch (e, stackTrace) {
67+
_log.severe("Error during app resume", e, stackTrace);
68+
} finally {
69+
if (!_resumeOperation!.isCompleted) {
70+
_resumeOperation!.complete();
71+
}
72+
_resumeOperation = null;
73+
}
74+
}
75+
76+
Future<void> _performResume() async {
4677
// no need to resume because app was never really paused
4778
if (!_wasPaused) return;
4879
_wasPaused = false;
@@ -53,9 +84,7 @@ class AppLifeCycleNotifier extends StateNotifier<AppLifeCycleEnum> {
5384
if (isAuthenticated) {
5485
// switch endpoint if needed
5586
final endpoint = await _ref.read(authProvider.notifier).setOpenApiServiceEndpoint();
56-
if (kDebugMode) {
57-
debugPrint("Using server URL: $endpoint");
58-
}
87+
_log.info("Using server URL: $endpoint");
5988

6089
if (!Store.isBetaTimelineEnabled) {
6190
final permission = _ref.watch(galleryPermissionNotifier);
@@ -81,63 +110,118 @@ class AppLifeCycleNotifier extends StateNotifier<AppLifeCycleEnum> {
81110
break;
82111
}
83112
} else {
84-
_ref.read(backupProvider.notifier).cancelBackup();
85-
final lockManager = _ref.read(isolateLockManagerProvider(kIsolateLockManagerPort));
113+
_ref.read(websocketProvider.notifier).connect();
114+
await _handleBetaTimelineResume();
115+
}
86116

87-
lockManager.requestHolderToClose();
88-
debugPrint("Requested lock holder to close on resume");
89-
await lockManager.acquireLock();
90-
debugPrint("Lock acquired for background sync on resume");
117+
await _ref.read(notificationPermissionProvider.notifier).getNotificationPermission();
91118

92-
final backgroundManager = _ref.read(backgroundSyncProvider);
93-
// Ensure proper cleanup before starting new background tasks
94-
try {
95-
await Future.wait([
96-
Future(() async {
97-
await backgroundManager.syncLocal();
98-
Logger("AppLifeCycleNotifier").fine("Hashing assets after syncLocal");
99-
// Check if app is still active before hashing
100-
if ([AppLifeCycleEnum.resumed, AppLifeCycleEnum.active].contains(state)) {
101-
await backgroundManager.hashAssets();
102-
}
103-
}),
104-
backgroundManager.syncRemote(),
105-
]).then((_) async {
106-
final isEnableBackup = _ref.read(appSettingsServiceProvider).getSetting(AppSettingsEnum.enableBackup);
119+
await _ref.read(galleryPermissionNotifier.notifier).getGalleryPermissionStatus();
107120

108-
final isAlbumLinkedSyncEnable = _ref.read(appSettingsServiceProvider).getSetting(AppSettingsEnum.syncAlbums);
121+
if (!Store.isBetaTimelineEnabled) {
122+
await _ref.read(iOSBackgroundSettingsProvider.notifier).refresh();
109123

110-
if (isEnableBackup) {
111-
final currentUser = _ref.read(currentUserProvider);
112-
if (currentUser == null) {
113-
return;
114-
}
124+
_ref.invalidate(memoryFutureProvider);
125+
}
126+
}
115127

116-
await _ref.read(driftBackupProvider.notifier).handleBackupResume(currentUser.id);
117-
}
128+
Future<void> _handleBetaTimelineResume() async {
129+
_ref.read(backupProvider.notifier).cancelBackup();
130+
final lockManager = _ref.read(isolateLockManagerProvider(kIsolateLockManagerPort));
118131

119-
if (isAlbumLinkedSyncEnable) {
120-
await backgroundManager.syncLinkedAlbum();
121-
}
122-
});
123-
} catch (e, stackTrace) {
124-
Logger("AppLifeCycleNotifier").severe("Error during background sync", e, stackTrace);
125-
}
132+
// Give isolates time to complete any ongoing database transactions
133+
await Future.delayed(const Duration(milliseconds: 500));
134+
135+
lockManager.requestHolderToClose();
136+
137+
// Add timeout to prevent deadlock on lock acquisition
138+
try {
139+
await lockManager.acquireLock().timeout(
140+
const Duration(seconds: 10),
141+
onTimeout: () {
142+
_log.warning("Lock acquisition timed out, proceeding without lock");
143+
throw TimeoutException("Lock acquisition timed out", const Duration(seconds: 10));
144+
},
145+
);
146+
} catch (e) {
147+
_log.warning("Failed to acquire lock: $e");
148+
return;
126149
}
127150

128-
_ref.read(websocketProvider.notifier).connect();
151+
final backgroundManager = _ref.read(backgroundSyncProvider);
152+
final isAlbumLinkedSyncEnable = _ref.read(appSettingsServiceProvider).getSetting(AppSettingsEnum.syncAlbums);
129153

130-
await _ref.read(notificationPermissionProvider.notifier).getNotificationPermission();
154+
try {
155+
// Run operations sequentially with state checks and error handling for each
156+
if (_shouldContinueOperation()) {
157+
try {
158+
await backgroundManager.syncLocal();
159+
} catch (e, stackTrace) {
160+
_log.warning("Failed syncLocal: $e", e, stackTrace);
161+
}
162+
}
131163

132-
await _ref.read(galleryPermissionNotifier.notifier).getGalleryPermissionStatus();
164+
// Check if app is still active before hashing
165+
if (_shouldContinueOperation()) {
166+
try {
167+
await backgroundManager.hashAssets();
168+
} catch (e, stackTrace) {
169+
_log.warning("Failed hashAssets: $e", e, stackTrace);
170+
}
171+
}
133172

134-
if (!Store.isBetaTimelineEnabled) {
135-
await _ref.read(iOSBackgroundSettingsProvider.notifier).refresh();
173+
// Check if app is still active before remote sync
174+
if (_shouldContinueOperation()) {
175+
try {
176+
await backgroundManager.syncRemote();
177+
} catch (e, stackTrace) {
178+
_log.warning("Failed syncRemote: $e", e, stackTrace);
179+
}
136180

137-
_ref.invalidate(memoryFutureProvider);
181+
if (isAlbumLinkedSyncEnable && _shouldContinueOperation()) {
182+
try {
183+
await backgroundManager.syncLinkedAlbum();
184+
} catch (e, stackTrace) {
185+
_log.warning("Failed syncLinkedAlbum: $e", e, stackTrace);
186+
}
187+
}
188+
}
189+
190+
// Handle backup resume only if still active
191+
if (_shouldContinueOperation()) {
192+
final isEnableBackup = _ref.read(appSettingsServiceProvider).getSetting(AppSettingsEnum.enableBackup);
193+
194+
if (isEnableBackup) {
195+
final currentUser = _ref.read(currentUserProvider);
196+
if (currentUser != null) {
197+
try {
198+
await _ref.read(driftBackupProvider.notifier).handleBackupResume(currentUser.id);
199+
_log.fine("Completed backup resume");
200+
} catch (e, stackTrace) {
201+
_log.warning("Failed backup resume: $e", e, stackTrace);
202+
}
203+
}
204+
}
205+
}
206+
} catch (e, stackTrace) {
207+
_log.severe("Error during background sync", e, stackTrace);
208+
} finally {
209+
// Ensure lock is released even if operations fail
210+
try {
211+
lockManager.releaseLock();
212+
_log.fine("Lock released after background sync operations");
213+
} catch (lockError) {
214+
_log.warning("Failed to release lock after error: $lockError");
215+
}
138216
}
139217
}
140218

219+
// Helper method to check if operations should continue
220+
bool _shouldContinueOperation() {
221+
return [AppLifeCycleEnum.resumed, AppLifeCycleEnum.active].contains(state) &&
222+
(_resumeOperation?.isCompleted == false || _resumeOperation == null);
223+
}
224+
141225
void handleAppInactivity() {
142226
state = AppLifeCycleEnum.inactive;
143227
// do not stop/clean up anything on inactivity: issued on every orientation change
@@ -147,6 +231,32 @@ class AppLifeCycleNotifier extends StateNotifier<AppLifeCycleEnum> {
147231
state = AppLifeCycleEnum.paused;
148232
_wasPaused = true;
149233

234+
// Prevent overlapping pause operations
235+
if (_pauseOperation != null && !_pauseOperation!.isCompleted) {
236+
await _pauseOperation!.future;
237+
return;
238+
}
239+
240+
// Cancel any ongoing resume operation
241+
if (_resumeOperation != null && !_resumeOperation!.isCompleted) {
242+
_resumeOperation!.complete();
243+
}
244+
245+
_pauseOperation = Completer<void>();
246+
247+
try {
248+
await _performPause();
249+
} catch (e, stackTrace) {
250+
_log.severe("Error during app pause", e, stackTrace);
251+
} finally {
252+
if (!_pauseOperation!.isCompleted) {
253+
_pauseOperation!.complete();
254+
}
255+
_pauseOperation = null;
256+
}
257+
}
258+
259+
Future<void> _performPause() async {
150260
if (_ref.read(authProvider).isAuthenticated) {
151261
if (!Store.isBetaTimelineEnabled) {
152262
// Do not cancel backup if manual upload is in progress
@@ -155,20 +265,34 @@ class AppLifeCycleNotifier extends StateNotifier<AppLifeCycleEnum> {
155265
}
156266
} else {
157267
final backgroundManager = _ref.read(backgroundSyncProvider);
158-
await backgroundManager.cancel();
159-
await backgroundManager.cancelLocal();
160-
_ref.read(isolateLockManagerProvider(kIsolateLockManagerPort)).releaseLock();
161-
debugPrint("Lock released on app pause");
268+
269+
// Cancel operations with extended timeout to allow database transactions to complete
270+
try {
271+
await Future.wait([
272+
backgroundManager.cancel().timeout(const Duration(seconds: 10)),
273+
backgroundManager.cancelLocal().timeout(const Duration(seconds: 10)),
274+
]).timeout(const Duration(seconds: 15));
275+
276+
// Give additional time for isolates to clean up database connections
277+
await Future.delayed(const Duration(milliseconds: 1000));
278+
} catch (e) {
279+
_log.warning("Timeout during background cancellation: $e");
280+
}
281+
282+
// Always release the lock, even if cancellation failed
283+
try {
284+
_ref.read(isolateLockManagerProvider(kIsolateLockManagerPort)).releaseLock();
285+
} catch (e) {
286+
_log.warning("Failed to release lock on pause: $e");
287+
}
162288
}
163289

164290
_ref.read(websocketProvider.notifier).disconnect();
165291
}
166292

167293
try {
168294
LogService.I.flush();
169-
} catch (e) {
170-
// Ignore flush errors during pause
171-
}
295+
} catch (_) {}
172296
}
173297

174298
Future<void> handleAppDetached() async {
@@ -177,19 +301,15 @@ class AppLifeCycleNotifier extends StateNotifier<AppLifeCycleEnum> {
177301
// Flush logs before closing database
178302
try {
179303
LogService.I.flush();
180-
} catch (e) {
181-
// Ignore flush errors during shutdown
182-
}
304+
} catch (_) {}
183305

184306
// Close Isar database safely
185307
try {
186308
final isar = Isar.getInstance();
187309
if (isar != null && isar.isOpen) {
188310
await isar.close();
189311
}
190-
} catch (e) {
191-
// Ignore close errors during shutdown
192-
}
312+
} catch (_) {}
193313

194314
if (Store.isBetaTimelineEnabled) {
195315
_ref.read(isolateLockManagerProvider(kIsolateLockManagerPort)).releaseLock();
@@ -199,9 +319,7 @@ class AppLifeCycleNotifier extends StateNotifier<AppLifeCycleEnum> {
199319
// no guarantee this is called at all
200320
try {
201321
_ref.read(manualUploadProvider.notifier).cancelBackup();
202-
} catch (e) {
203-
// Ignore errors during shutdown
204-
}
322+
} catch (_) {}
205323
}
206324

207325
void handleAppHidden() {

‎mobile/lib/widgets/common/immich_sliver_app_bar.dart‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -296,7 +296,7 @@ class _SyncStatusIndicatorState extends ConsumerState<_SyncStatusIndicator> with
296296
@override
297297
Widget build(BuildContext context) {
298298
final syncStatus = ref.watch(syncStatusProvider);
299-
final isSyncing = syncStatus.isRemoteSyncing;
299+
final isSyncing = syncStatus.isRemoteSyncing || syncStatus.isLocalSyncing;
300300

301301
// Control animations based on sync status
302302
if (isSyncing) {

0 commit comments

Comments
 (0)