diff --git a/mobile/lib/services/local_sync_service.dart b/mobile/lib/services/local_sync_service.dart index 1915ac30c2..ceb070011b 100644 --- a/mobile/lib/services/local_sync_service.dart +++ b/mobile/lib/services/local_sync_service.dart @@ -23,6 +23,7 @@ import "package:photos/utils/debouncer.dart"; import "package:photos/utils/photo_manager_util.dart"; import "package:photos/utils/sqlite_util.dart"; import 'package:shared_preferences/shared_preferences.dart'; +import 'package:synchronized/synchronized.dart'; import 'package:tuple/tuple.dart'; class LocalSyncService { @@ -31,6 +32,7 @@ class LocalSyncService { late SharedPreferences _prefs; Completer? _existingSync; late Debouncer _changeCallbackDebouncer; + final Lock _lock = Lock(); static const kDbUpdationTimeKey = "db_updation_time"; static const kHasCompletedFirstImportKey = "has_completed_firstImport"; @@ -77,50 +79,57 @@ class LocalSyncService { } _existingSync = Completer(); final int ownerID = Configuration.instance.getUserID()!; - final existingLocalFileIDs = await _db.getExistingLocalFileIDs(ownerID); - _logger.info("${existingLocalFileIDs.length} localIDs were discovered"); + + // We use a lock to prevent synchronisation to occur while it is downloading + // as this introduces wrong entry in FilesDB due to race condition + // This is a fix for https://github.com/ente-io/ente/issues/4296 + await _lock.synchronized(() async { + final existingLocalFileIDs = await _db.getExistingLocalFileIDs(ownerID); + _logger.info("${existingLocalFileIDs.length} localIDs were discovered"); - final syncStartTime = DateTime.now().microsecondsSinceEpoch; - final lastDBUpdationTime = _prefs.getInt(kDbUpdationTimeKey) ?? 0; - final startTime = DateTime.now().microsecondsSinceEpoch; - if (lastDBUpdationTime != 0) { - await _loadAndStoreDiff( - existingLocalFileIDs, - fromTime: lastDBUpdationTime, - toTime: syncStartTime, - ); - } else { - // Load from 0 - 01.01.2010 - Bus.instance.fire(SyncStatusUpdate(SyncStatus.startedFirstGalleryImport)); - var startTime = 0; - var toYear = 2010; - var toTime = DateTime(toYear).microsecondsSinceEpoch; - while (toTime < syncStartTime) { + final syncStartTime = DateTime.now().microsecondsSinceEpoch; + final lastDBUpdationTime = _prefs.getInt(kDbUpdationTimeKey) ?? 0; + final startTime = DateTime.now().microsecondsSinceEpoch; + if (lastDBUpdationTime != 0) { + await _loadAndStoreDiff( + existingLocalFileIDs, + fromTime: lastDBUpdationTime, + toTime: syncStartTime, + ); + } else { + // Load from 0 - 01.01.2010 + Bus.instance.fire(SyncStatusUpdate(SyncStatus.startedFirstGalleryImport)); + var startTime = 0; + var toYear = 2010; + var toTime = DateTime(toYear).microsecondsSinceEpoch; + while (toTime < syncStartTime) { + await _loadAndStoreDiff( + existingLocalFileIDs, + fromTime: startTime, + toTime: toTime, + ); + startTime = toTime; + toYear++; + toTime = DateTime(toYear).microsecondsSinceEpoch; + } await _loadAndStoreDiff( existingLocalFileIDs, fromTime: startTime, - toTime: toTime, + toTime: syncStartTime, ); - startTime = toTime; - toYear++; - toTime = DateTime(toYear).microsecondsSinceEpoch; } - await _loadAndStoreDiff( - existingLocalFileIDs, - fromTime: startTime, - toTime: syncStartTime, - ); - } - if (!hasCompletedFirstImport()) { - await _prefs.setBool(kHasCompletedFirstImportKey, true); - await _refreshDeviceFolderCountAndCover(isFirstSync: true); - _logger.fine("first gallery import finished"); - Bus.instance - .fire(SyncStatusUpdate(SyncStatus.completedFirstGalleryImport)); - } - final endTime = DateTime.now().microsecondsSinceEpoch; - final duration = Duration(microseconds: endTime - startTime); - _logger.info("Load took " + duration.inMilliseconds.toString() + "ms"); + if (!hasCompletedFirstImport()) { + await _prefs.setBool(kHasCompletedFirstImportKey, true); + await _refreshDeviceFolderCountAndCover(isFirstSync: true); + _logger.fine("first gallery import finished"); + Bus.instance + .fire(SyncStatusUpdate(SyncStatus.completedFirstGalleryImport)); + } + final endTime = DateTime.now().microsecondsSinceEpoch; + final duration = Duration(microseconds: endTime - startTime); + _logger.info("Load took " + duration.inMilliseconds.toString() + "ms"); + }); + _existingSync?.complete(); _existingSync = null; } @@ -240,6 +249,10 @@ class LocalSyncService { } } + Lock getLock() { + return _lock; + } + bool hasGrantedPermissions() { return _prefs.getBool(kHasGrantedPermissionsKey) ?? false; } @@ -361,7 +374,7 @@ class LocalSyncService { unawaited(checkAndSync()); }); }); - PhotoManager.startChangeNotify(); + PhotoManagerSafe.startChangeNotify(null); } Future checkAndSync() async { diff --git a/mobile/lib/ui/tools/editor/image_editor_page.dart b/mobile/lib/ui/tools/editor/image_editor_page.dart index 2ef93601f2..a24aa8bec7 100644 --- a/mobile/lib/ui/tools/editor/image_editor_page.dart +++ b/mobile/lib/ui/tools/editor/image_editor_page.dart @@ -27,6 +27,7 @@ import 'package:photos/ui/tools/editor/filtered_image.dart'; import 'package:photos/ui/viewer/file/detail_page.dart'; import 'package:photos/utils/dialog_util.dart'; import 'package:photos/utils/navigation_util.dart'; +import "package:photos/utils/photo_manager_util.dart"; import 'package:photos/utils/toast_util.dart'; import 'package:syncfusion_flutter_core/theme.dart'; import 'package:syncfusion_flutter_sliders/sliders.dart'; @@ -358,7 +359,7 @@ class _ImageEditorPageState extends State { ".JPEG"; //Disabling notifications for assets changing to insert the file into //files db before triggering a sync. - await PhotoManager.stopChangeNotify(); + await PhotoManagerSafe.stopChangeNotify(widget.originalFile.title!); final AssetEntity? newAsset = await (PhotoManager.editor.saveImage(result, filename: fileName)); final newFile = await ente.EnteFile.fromAsset( @@ -410,7 +411,7 @@ class _ImageEditorPageState extends State { showToast(context, S.of(context).oopsCouldNotSaveEdits); _logger.severe(e, s); } finally { - await PhotoManager.startChangeNotify(); + await PhotoManagerSafe.startChangeNotify(widget.originalFile.title!); } } diff --git a/mobile/lib/ui/tools/editor/video_editor_page.dart b/mobile/lib/ui/tools/editor/video_editor_page.dart index 392d93034c..34f0e9ba97 100644 --- a/mobile/lib/ui/tools/editor/video_editor_page.dart +++ b/mobile/lib/ui/tools/editor/video_editor_page.dart @@ -24,6 +24,7 @@ import "package:photos/ui/tools/editor/video_trim_page.dart"; import "package:photos/ui/viewer/file/detail_page.dart"; import "package:photos/utils/dialog_util.dart"; import "package:photos/utils/navigation_util.dart"; +import "package:photos/utils/photo_manager_util.dart"; import "package:photos/utils/toast_util.dart"; import "package:video_editor/video_editor.dart"; @@ -238,7 +239,7 @@ class _VideoEditorPageState extends State { ".mp4"; //Disabling notifications for assets changing to insert the file into //files db before triggering a sync. - await PhotoManager.stopChangeNotify(); + await PhotoManagerSafe.stopChangeNotify(widget.file.title!); try { final AssetEntity? newAsset = @@ -299,6 +300,8 @@ class _VideoEditorPageState extends State { ); } catch (_) { await dialog.hide(); + } finally { + await PhotoManagerSafe.startChangeNotify(widget.file.title!); } } } diff --git a/mobile/lib/ui/viewer/actions/file_selection_actions_widget.dart b/mobile/lib/ui/viewer/actions/file_selection_actions_widget.dart index 00c06bb2ef..dc97628682 100644 --- a/mobile/lib/ui/viewer/actions/file_selection_actions_widget.dart +++ b/mobile/lib/ui/viewer/actions/file_selection_actions_widget.dart @@ -822,9 +822,12 @@ class _FileSelectionActionsWidgetState } Future _download(List files) async { + final totalFiles = files.length; + int downloadedFiles = 0; + final dialog = createProgressDialog( context, - S.of(context).downloading, + S.of(context).downloading + "$downloadedFiles/$totalFiles", isDismissible: true, ); await dialog.show(); @@ -832,7 +835,12 @@ class _FileSelectionActionsWidgetState final futures = []; for (final file in files) { if (file.localID == null) { - futures.add(downloadToGallery(file)); + futures.add( + downloadToGallery(file).then((_) { + downloadedFiles++; + dialog.update(message: S.of(context).downloading + " ($downloadedFiles/$totalFiles)"); + }), + ); } } await Future.wait(futures); diff --git a/mobile/lib/utils/file_download_util.dart b/mobile/lib/utils/file_download_util.dart index 1014005c94..1841fcfca5 100644 --- a/mobile/lib/utils/file_download_util.dart +++ b/mobile/lib/utils/file_download_util.dart @@ -21,6 +21,7 @@ import "package:photos/utils/data_util.dart"; import "package:photos/utils/fake_progress.dart"; import "package:photos/utils/file_key.dart"; import "package:photos/utils/file_util.dart"; +import "package:photos/utils/photo_manager_util.dart"; final _logger = Logger("file_download_util"); @@ -189,50 +190,54 @@ Future downloadToGallery(EnteFile file) async { final bool downloadLivePhotoOnDroid = type == FileType.livePhoto && Platform.isAndroid; AssetEntity? savedAsset; - final File? fileToSave = await getFile(file); - //Disabling notifications for assets changing to insert the file into - //files db before triggering a sync. - await PhotoManager.stopChangeNotify(); - if (type == FileType.image) { - savedAsset = await PhotoManager.editor - .saveImageWithPath(fileToSave!.path, title: file.title!); - } else if (type == FileType.video) { - savedAsset = - await PhotoManager.editor.saveVideo(fileToSave!, title: file.title!); - } else if (type == FileType.livePhoto) { - final File? liveVideoFile = - await getFileFromServer(file, liveVideo: true); - if (liveVideoFile == null) { - throw AssertionError("Live video can not be null"); + // We use a lock to prevent synchronisation to occur while it is downloading + // as this introduces wrong entry in FilesDB due to race condition + // This is a fix for https://github.com/ente-io/ente/issues/4296 + await LocalSyncService.instance.getLock().synchronized(() async { + final File? fileToSave = await getFile(file); + //Disabling notifications for assets changing to insert the file into + //files db before triggering a sync. + await PhotoManagerSafe.stopChangeNotify(file.generatedID.toString()); + if (type == FileType.image) { + savedAsset = await PhotoManager.editor + .saveImageWithPath(fileToSave!.path, title: file.title!); + } else if (type == FileType.video) { + savedAsset = + await PhotoManager.editor.saveVideo(fileToSave!, title: file.title!); + } else if (type == FileType.livePhoto) { + final File? liveVideoFile = + await getFileFromServer(file, liveVideo: true); + if (liveVideoFile == null) { + throw AssertionError("Live video can not be null"); + } + if (downloadLivePhotoOnDroid) { + await _saveLivePhotoOnDroid(fileToSave!, liveVideoFile, file); + } else { + savedAsset = await PhotoManager.editor.darwin.saveLivePhoto( + imageFile: fileToSave!, + videoFile: liveVideoFile, + title: file.title!, + ); + } } - if (downloadLivePhotoOnDroid) { - await _saveLivePhotoOnDroid(fileToSave!, liveVideoFile, file); - } else { - savedAsset = await PhotoManager.editor.darwin.saveLivePhoto( - imageFile: fileToSave!, - videoFile: liveVideoFile, - title: file.title!, + if (savedAsset != null) { + file.localID = savedAsset!.id; + await FilesDB.instance.insert(file); + Bus.instance.fire( + LocalPhotosUpdatedEvent( + [file], + source: "download", + ), ); + } else if (!downloadLivePhotoOnDroid && savedAsset == null) { + _logger.severe('Failed to save assert of type $type'); } - } - - if (savedAsset != null) { - file.localID = savedAsset.id; - await FilesDB.instance.insert(file); - Bus.instance.fire( - LocalPhotosUpdatedEvent( - [file], - source: "download", - ), - ); - } else if (!downloadLivePhotoOnDroid && savedAsset == null) { - _logger.severe('Failed to save assert of type $type'); - } + }); } catch (e) { _logger.severe("Failed to save file", e); rethrow; } finally { - await PhotoManager.startChangeNotify(); + await PhotoManagerSafe.startChangeNotify(file.generatedID.toString()); LocalSyncService.instance.checkAndSync().ignore(); } } diff --git a/mobile/lib/utils/photo_manager_util.dart b/mobile/lib/utils/photo_manager_util.dart index 273d0b362e..948d1cf863 100644 --- a/mobile/lib/utils/photo_manager_util.dart +++ b/mobile/lib/utils/photo_manager_util.dart @@ -1,4 +1,7 @@ +import 'dart:async'; +import "package:logging/logging.dart"; import "package:photo_manager/photo_manager.dart"; +import 'package:synchronized/synchronized.dart'; Future requestPhotoMangerPermissions() { return PhotoManager.requestPermissionExtend( @@ -10,3 +13,128 @@ Future requestPhotoMangerPermissions() { ), ); } + +final _logger = Logger("PhotoManagerUtil"); +// This is a wrapper for safe handling of PhotoManager.startChangeNotify() and +// PhotoManager.stopChangeNotify(). Since PhotoManager is globally shared, we want +// to make sure no notification is sent while it should not. The logic is that it will +// only start if no other asset (or task) requested to stop changes, or if the expiration +// time for the asset (task) expired. '_processingAssets' should be seen as a queue of +// open requests. +class PhotoManagerSafe { + // Tracks processing assets with their expiration times + static final Map _processingAssets = {}; + + // Timer for monitoring asset processing + static Timer? _expirationTimer; + + // Synchronization lock + static final _lock = Lock(); + + // Estimate processing duration based on file size + static Duration _estimateProcessingDuration(int fileSize) { + final estimatedSeconds = (fileSize / (1024 * 1024)).ceil() * 2; + return Duration( + seconds: estimatedSeconds.clamp(5, 120), + ); + } + + // Manage asset processing state. Lock ensures no start/stop is performed + // at the same time. + static Future manageAssetProcessing({ + required String? assetId, + required bool isStarting, + int? fileSize, + }) async { + await _lock.synchronized(() async { + try { + if (isStarting) { + // Remove the asset from processing only if assetId is not null + if (assetId != null) { + _processingAssets.remove(assetId); + } + + // Restart change notify only if no assets are processing and no stop was requested + if (_processingAssets.isEmpty) { + await PhotoManager.startChangeNotify(); + } + + _stopExpirationMonitoringIfEmpty(); + } else { + // Stopping the asset + final duration = _estimateProcessingDuration( + fileSize ?? 10 * 1024 * 1024, // 10MB default + ); + + // First asset to request stop + if (_processingAssets.isEmpty) { + await PhotoManager.stopChangeNotify(); + } + + // Track the processing asset with expiration + _processingAssets[assetId] = DateTime.now().add(duration); + + _startOrContinueExpirationMonitoring(); + } + } catch (e, stackTrace) { + _logger.severe( + "${isStarting ? 'Start' : 'Stop'}ChangeNotify error for ID $assetId", + e, + stackTrace, + ); + rethrow; + } + }); + } + + // Start or continue the expiration monitoring timer + static void _startOrContinueExpirationMonitoring() { + if (_expirationTimer != null && _expirationTimer!.isActive) return; + + _expirationTimer = Timer.periodic(const Duration(seconds: 1), (timer) { + final now = DateTime.now(); + + // Remove expired assets + _processingAssets.removeWhere((assetId, expiresAt) { + final bool isExpired = expiresAt.isBefore(now); + if (isExpired) { + } + return isExpired; + }); + + // Handle asset processing completion + if (_processingAssets.isEmpty) { + + // Start ChangeNotify + try { + PhotoManager.startChangeNotify(); + } catch (e, stackTrace) { + _logger.severe("Error restarting change notify", e, stackTrace); + } + + _stopExpirationMonitoringIfEmpty(); + } + }); + } + + // Stop the expiration monitoring timer if no assets are being processed + static void _stopExpirationMonitoringIfEmpty() { + if (_processingAssets.isEmpty) { + _expirationTimer?.cancel(); + _expirationTimer = null; + } + } + + static Future stopChangeNotify(String? assetId, {int? fileSize}) => + manageAssetProcessing( + assetId: assetId, + isStarting: false, + fileSize: fileSize, + ); + + static Future startChangeNotify(String? assetId) => + manageAssetProcessing( + assetId: assetId, + isStarting: true, + ); +} \ No newline at end of file