From e8d8e8b160b5286e9215d8a17004968627b4afdc Mon Sep 17 00:00:00 2001 From: otsmr Date: Fri, 24 Apr 2026 23:14:48 +0200 Subject: [PATCH] fixes multiple race condition issues --- .../services/api/mediafiles/download.api.dart | 142 ++++++++++-------- .../callback_dispatcher.background.dart | 2 +- ...ccess.dart => exclusive_access.utils.dart} | 6 +- lib/src/utils/keyvalue.dart | 66 +++++--- lib/src/utils/log.dart | 2 +- 5 files changed, 128 insertions(+), 90 deletions(-) rename lib/src/utils/{exclusive_access.dart => exclusive_access.utils.dart} (91%) diff --git a/lib/src/services/api/mediafiles/download.api.dart b/lib/src/services/api/mediafiles/download.api.dart index dac06e9f..89a8b757 100644 --- a/lib/src/services/api/mediafiles/download.api.dart +++ b/lib/src/services/api/mediafiles/download.api.dart @@ -15,6 +15,7 @@ import 'package:twonly/src/database/twonly.db.dart'; import 'package:twonly/src/model/protobuf/client/generated/messages.pbserver.dart'; import 'package:twonly/src/services/api/messages.api.dart'; import 'package:twonly/src/services/mediafiles/mediafile.service.dart'; +import 'package:twonly/src/utils/exclusive_access.utils.dart'; import 'package:twonly/src/utils/log.dart'; import 'package:twonly/src/utils/misc.dart'; @@ -158,7 +159,8 @@ Future handleDownloadStatusUpdate(TaskStatusUpdate update) async { } } -Mutex protectDownload = Mutex(); +Mutex _protectDownload = Mutex(); +Mutex _protectDecryption = Mutex(); Future startDownloadMedia(MediaFile media, bool force) async { final mediaService = MediaFileService(media); @@ -175,7 +177,7 @@ Future startDownloadMedia(MediaFile media, bool force) async { return; } - final isBlocked = await protectDownload.protect(() async { + final isBlocked = await _protectDownload.protect(() async { final msg = await twonlyDB.mediaFilesDao.getMediaFileById(media.mediaId); if (msg == null || msg.downloadState != DownloadState.pending) { @@ -285,66 +287,86 @@ Future requestMediaReupload(String mediaId) async { } Future handleEncryptedFile(String mediaId) async { - final mediaService = await MediaFileService.fromMediaId(mediaId); - if (mediaService == null) { - Log.error('Media file not found in database.'); - return; - } + await exclusiveAccess( + lockName: 'decryption-$mediaId', + mutex: _protectDecryption, + action: () async { + final mediaService = await MediaFileService.fromMediaId(mediaId); + if (mediaService == null) { + Log.error('Media file not found in database.'); + return; + } - await twonlyDB.mediaFilesDao.updateMedia( - mediaId, - const MediaFilesCompanion( - downloadState: Value(DownloadState.downloaded), - ), + if (mediaService.mediaFile.downloadState == DownloadState.ready) { + Log.info('Decryption of $mediaId already finished.'); + return; + } + + if (!mediaService.encryptedPath.existsSync()) { + Log.warn( + 'Encrypted media file $mediaId does not exist anymore. Decryption probably already finished.', + ); + return; + } + + late Uint8List encryptedBytes; + try { + encryptedBytes = await mediaService.encryptedPath.readAsBytes(); + } catch (e) { + Log.error('Could not read encrypted media file: $e'); + await requestMediaReupload(mediaId); + return; + } + + await twonlyDB.mediaFilesDao.updateMedia( + mediaId, + const MediaFilesCompanion( + downloadState: Value(DownloadState.downloaded), + ), + ); + + try { + final chacha20 = FlutterChacha20.poly1305Aead(); + final secretKeyData = SecretKeyData( + mediaService.mediaFile.encryptionKey!, + ); + + final secretBox = SecretBox( + encryptedBytes, + nonce: mediaService.mediaFile.encryptionNonce!, + mac: Mac(mediaService.mediaFile.encryptionMac!), + ); + + final plaintextBytes = await chacha20.decrypt( + secretBox, + secretKey: secretKeyData, + ); + + final rawMediaBytes = Uint8List.fromList(plaintextBytes); + + await mediaService.tempPath.writeAsBytes(rawMediaBytes); + } catch (e) { + Log.error( + 'Could not decrypt the media file. Requesting a new upload.', + ); + await requestMediaReupload(mediaId); + return; + } + + await twonlyDB.mediaFilesDao.updateMedia( + mediaId, + const MediaFilesCompanion( + downloadState: Value(DownloadState.ready), + ), + ); + + Log.info('Decryption of $mediaId was successful'); + + mediaService.encryptedPath.deleteSync(); + + unawaited(apiService.downloadDone(mediaService.mediaFile.downloadToken!)); + }, ); - - late Uint8List encryptedBytes; - try { - encryptedBytes = await mediaService.encryptedPath.readAsBytes(); - } catch (e) { - Log.error('Could not read encrypted media file: $e'); - await requestMediaReupload(mediaId); - return; - } - - try { - final chacha20 = FlutterChacha20.poly1305Aead(); - final secretKeyData = SecretKeyData(mediaService.mediaFile.encryptionKey!); - - final secretBox = SecretBox( - encryptedBytes, - nonce: mediaService.mediaFile.encryptionNonce!, - mac: Mac(mediaService.mediaFile.encryptionMac!), - ); - - final plaintextBytes = await chacha20.decrypt( - secretBox, - secretKey: secretKeyData, - ); - - final rawMediaBytes = Uint8List.fromList(plaintextBytes); - - await mediaService.tempPath.writeAsBytes(rawMediaBytes); - } catch (e) { - Log.error( - 'Could not decrypt the media file. Requesting a new upload.', - ); - await requestMediaReupload(mediaId); - return; - } - - await twonlyDB.mediaFilesDao.updateMedia( - mediaId, - const MediaFilesCompanion( - downloadState: Value(DownloadState.ready), - ), - ); - - Log.info('Decryption of $mediaId was successful'); - - mediaService.encryptedPath.deleteSync(); - - unawaited(apiService.downloadDone(mediaService.mediaFile.downloadToken!)); } Future makeMigrationToVersion91() async { diff --git a/lib/src/services/background/callback_dispatcher.background.dart b/lib/src/services/background/callback_dispatcher.background.dart index f0e8d4ae..249a10fe 100644 --- a/lib/src/services/background/callback_dispatcher.background.dart +++ b/lib/src/services/background/callback_dispatcher.background.dart @@ -6,7 +6,7 @@ import 'package:twonly/locator.dart'; import 'package:twonly/main.dart'; import 'package:twonly/src/constants/keyvalue.keys.dart'; import 'package:twonly/src/services/api/mediafiles/upload.api.dart'; -import 'package:twonly/src/utils/exclusive_access.dart'; +import 'package:twonly/src/utils/exclusive_access.utils.dart'; import 'package:twonly/src/utils/keyvalue.dart'; import 'package:twonly/src/utils/log.dart'; import 'package:workmanager/workmanager.dart'; diff --git a/lib/src/utils/exclusive_access.dart b/lib/src/utils/exclusive_access.utils.dart similarity index 91% rename from lib/src/utils/exclusive_access.dart rename to lib/src/utils/exclusive_access.utils.dart index fb64a397..f39ce62e 100644 --- a/lib/src/utils/exclusive_access.dart +++ b/lib/src/utils/exclusive_access.utils.dart @@ -8,7 +8,7 @@ Future exclusiveAccess({ required Future Function() action, required Mutex mutex, }) async { - final lockFile = File('${AppEnvironment.supportDir}/$lockName.lock'); + final lockFile = File('${AppEnvironment.cacheDir}/$lockName.lock'); return mutex.protect(() async { var lockAcquired = false; @@ -24,8 +24,8 @@ Future exclusiveAccess({ try { final stat = lockFile.statSync(); if (stat.type != FileSystemEntityType.notFound) { - final age = DateTime.now().difference(stat.modified).inMilliseconds; - if (age > 1000) { + final age = DateTime.now().difference(stat.modified).inSeconds; + if (age > 10) { lockFile.deleteSync(); continue; } diff --git a/lib/src/utils/keyvalue.dart b/lib/src/utils/keyvalue.dart index f970691b..b8f5ce5d 100644 --- a/lib/src/utils/keyvalue.dart +++ b/lib/src/utils/keyvalue.dart @@ -1,14 +1,27 @@ import 'dart:convert'; import 'dart:io'; + +import 'package:mutex/mutex.dart'; import 'package:twonly/globals.dart'; +import 'package:twonly/src/utils/exclusive_access.utils.dart'; import 'package:twonly/src/utils/log.dart'; class KeyValueStore { + static final Mutex _mutex = Mutex(); + static Future _getFilePath(String key) async { return File('${AppEnvironment.supportDir}/keyvalue/$key.json'); } - static Future delete(String key) async { + static Future _exclusive(String key, Future Function() action) { + return exclusiveAccess( + lockName: 'keyvalue-$key', + mutex: _mutex, + action: action, + ); + } + + static Future delete(String key) => _exclusive(key, () async { try { final file = await _getFilePath(key); if (file.existsSync()) { @@ -17,30 +30,33 @@ class KeyValueStore { } catch (e) { Log.error('Error deleting file: $e'); } - } + }); - static Future?> get(String key) async { - try { - final file = await _getFilePath(key); - if (file.existsSync()) { - final contents = await file.readAsString(); - return jsonDecode(contents) as Map; - } else { - return null; - } - } catch (e) { - Log.warn('Error reading file: $e'); - return null; - } - } + static Future?> get(String key) => + _exclusive(key, () async { + final file = await _getFilePath(key); + try { + if (file.existsSync()) { + final contents = await file.readAsString(); + return jsonDecode(contents) as Map; + } else { + return null; + } + } catch (e) { + Log.warn('Error reading file. Deleting it.: $e'); + file.deleteSync(); + return null; + } + }); - static Future put(String key, Map value) async { - try { - final file = await _getFilePath(key); - await file.parent.create(recursive: true); - await file.writeAsString(jsonEncode(value)); - } catch (e) { - Log.error('Error writing file: $e'); - } - } + static Future put(String key, Map value) => + _exclusive(key, () async { + try { + final file = await _getFilePath(key); + await file.parent.create(recursive: true); + await file.writeAsString(jsonEncode(value)); + } catch (e) { + Log.error('Error writing file: $e'); + } + }); } diff --git a/lib/src/utils/log.dart b/lib/src/utils/log.dart index 0d229cde..088812e6 100644 --- a/lib/src/utils/log.dart +++ b/lib/src/utils/log.dart @@ -6,7 +6,7 @@ import 'package:logging/logging.dart'; import 'package:mutex/mutex.dart'; import 'package:sentry_flutter/sentry_flutter.dart'; import 'package:twonly/globals.dart'; -import 'package:twonly/src/utils/exclusive_access.dart'; +import 'package:twonly/src/utils/exclusive_access.utils.dart'; class Log { static bool _isInitialized = false;