From c700626a66fea30c2c56e0abb24c6b9697ecbe1b Mon Sep 17 00:00:00 2001 From: grunch Date: Mon, 31 Aug 2026 22:04:39 -0300 Subject: [PATCH 1/4] perf: lazy notifiers for terminal orders and a bounded media cache Startup eagerly created an OrderNotifier (storage watcher, book listener) and a ChatRoomNotifier (full history decrypt) for EVERY session of the retention window, finished trades included - N was "every trade of the month", and none of it was ever disposed. Each chat notifier also held its decrypted media bytes until process exit. - App init now skips orders whose last stored message reports a terminal status (missing or ambiguous data stays eager); the non-autoDispose families still build those notifiers lazily the moment a screen watches them (trade detail, status filters, chat history). - MediaCacheMixin is byte-bounded (32 MB combined) with LRU eviction; evicted media re-decrypts on demand from the stored blob. --- lib/shared/mixins/media_cache_mixin.dart | 27 ++++++ lib/shared/providers/app_init_provider.dart | 19 +++++ .../lazy_terminal_and_media_cache_test.dart | 83 +++++++++++++++++++ 3 files changed, 129 insertions(+) create mode 100644 test/shared/lazy_terminal_and_media_cache_test.dart diff --git a/lib/shared/mixins/media_cache_mixin.dart b/lib/shared/mixins/media_cache_mixin.dart index 6b012150a..9fc1d59fe 100644 --- a/lib/shared/mixins/media_cache_mixin.dart +++ b/lib/shared/mixins/media_cache_mixin.dart @@ -5,15 +5,38 @@ import 'package:mostro_mobile/services/encrypted_file_upload_service.dart'; /// Shared media cache for decrypted images and files. /// Used by both ChatRoomNotifier (P2P) and DisputeChatNotifier. mixin MediaCacheMixin { + /// Combined byte budget for decrypted media. These notifiers live for the + /// whole app run, so an unbounded cache held every decrypted photo and + /// file forever; the oldest entries are evicted once the budget is hit + /// (they re-decrypt on demand from the stored blob). + static const int mediaCacheMaxBytes = 32 * 1024 * 1024; + final Map _imageCache = {}; final Map _imageMetadata = {}; final Map _fileCache = {}; final Map _fileMetadata = {}; + final List _mediaLru = []; + int _mediaBytes = 0; + + int get debugMediaCacheBytes => _mediaBytes; + + void _mediaTouch(String messageId, int addedBytes) { + _mediaLru.remove(messageId); + _mediaLru.add(messageId); + _mediaBytes += addedBytes; + while (_mediaBytes > mediaCacheMaxBytes && _mediaLru.isNotEmpty) { + final oldest = _mediaLru.removeAt(0); + _mediaBytes -= _imageCache.remove(oldest)?.length ?? 0; + _mediaBytes -= _fileCache.remove(oldest)?.length ?? 0; + } + } void cacheDecryptedImage( String messageId, Uint8List data, EncryptedImageUploadResult meta) { + final previous = _imageCache[messageId]?.length ?? 0; _imageCache[messageId] = data; _imageMetadata[messageId] = meta; + _mediaTouch(messageId, data.length - previous); } Uint8List? getCachedImage(String messageId) => _imageCache[messageId]; @@ -24,7 +47,9 @@ mixin MediaCacheMixin { void cacheDecryptedFile( String messageId, Uint8List? data, EncryptedFileUploadResult meta) { if (data != null) { + final previous = _fileCache[messageId]?.length ?? 0; _fileCache[messageId] = data; + _mediaTouch(messageId, data.length - previous); } _fileMetadata[messageId] = meta; } @@ -39,5 +64,7 @@ mixin MediaCacheMixin { _imageMetadata.clear(); _fileCache.clear(); _fileMetadata.clear(); + _mediaLru.clear(); + _mediaBytes = 0; } } diff --git a/lib/shared/providers/app_init_provider.dart b/lib/shared/providers/app_init_provider.dart index 1e59a5176..16261051f 100644 --- a/lib/shared/providers/app_init_provider.dart +++ b/lib/shared/providers/app_init_provider.dart @@ -1,3 +1,6 @@ +import 'package:mostro_mobile/shared/providers/mostro_storage_provider.dart'; +import 'package:mostro_mobile/data/models/order.dart'; +import 'package:mostro_mobile/data/models/mostro_message.dart'; import 'dart:async'; import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:mostro_mobile/core/config.dart'; @@ -53,9 +56,17 @@ final appInitializerProvider = FutureProvider((ref) async { ? null : DateTime.now().subtract(Duration(hours: expirationHours)); + final messageStorage = ref.read(mostroStorageProvider); for (final session in sessionManager.sessions) { if(session.orderId == null || (cutoff != null && session.startTime.isBefore(cutoff))) continue; + // Terminal orders initialize lazily when a screen watches them: an eager + // notifier per finished trade meant a storage watcher, a book listener + // and (for chats) a history decrypt alive until process exit. + final latest = + await messageStorage.getLatestMessageById(session.orderId!); + if (isTerminalOrderMessage(latest)) continue; + ref.read(orderNotifierProvider(session.orderId!).notifier); if (session.peer != null) { @@ -63,3 +74,11 @@ final appInitializerProvider = FutureProvider((ref) async { } } }); + +/// Whether the order's last stored message reports a terminal status. A +/// missing message or a non-order payload counts as live, so anything +/// ambiguous keeps today's eager behaviour. +bool isTerminalOrderMessage(MostroMessage? message) { + final order = message?.getPayload(); + return order != null && order.status.isTerminal; +} diff --git a/test/shared/lazy_terminal_and_media_cache_test.dart b/test/shared/lazy_terminal_and_media_cache_test.dart new file mode 100644 index 000000000..55fad4e69 --- /dev/null +++ b/test/shared/lazy_terminal_and_media_cache_test.dart @@ -0,0 +1,83 @@ +import 'dart:typed_data'; + +import 'package:flutter_test/flutter_test.dart'; +import 'package:mostro_mobile/data/models/enums/action.dart'; +import 'package:mostro_mobile/data/models/enums/order_type.dart'; +import 'package:mostro_mobile/data/models/enums/status.dart'; +import 'package:mostro_mobile/data/models/mostro_message.dart'; +import 'package:mostro_mobile/data/models/order.dart'; +import 'package:mostro_mobile/services/encrypted_image_upload_service.dart'; +import 'package:mostro_mobile/shared/mixins/media_cache_mixin.dart'; +import 'package:mostro_mobile/shared/providers/app_init_provider.dart'; + +/// Startup eagerly created an OrderNotifier (storage watcher + listeners) and +/// a ChatRoomNotifier (history decrypt) for EVERY session of the last 30 +/// days, terminal or not, and each chat notifier held its decrypted media +/// bytes until process exit. Terminal orders now initialize lazily and the +/// media cache is byte-bounded. +void main() { + MostroMessage withStatus(Status status) => MostroMessage( + action: Action.newOrder, + id: 'o1', + payload: Order( + kind: OrderType.sell, + status: status, + fiatCode: 'VES', + fiatAmount: 100, + paymentMethod: 'cash', + ), + ); + + group('isTerminalOrderMessage', () { + test('terminal statuses skip eager initialization', () { + expect(isTerminalOrderMessage(withStatus(Status.canceled)), isTrue); + expect(isTerminalOrderMessage(withStatus(Status.success)), isTrue); + expect( + isTerminalOrderMessage(withStatus(Status.canceledByAdmin)), isTrue); + }); + + test('live statuses keep eager initialization', () { + expect(isTerminalOrderMessage(withStatus(Status.pending)), isFalse); + expect(isTerminalOrderMessage(withStatus(Status.active)), isFalse); + expect(isTerminalOrderMessage(withStatus(Status.fiatSent)), isFalse); + }); + + test('no message or no order payload counts as live (conservative)', () { + expect(isTerminalOrderMessage(null), isFalse); + expect( + isTerminalOrderMessage(MostroMessage(action: Action.rate, id: 'o1')), + isFalse, + ); + }); + }); + + group('MediaCacheMixin byte bound', () { + test('evicts the oldest entries once the cap is exceeded', () { + final cache = _CacheHost(); + final chunk = Uint8List(MediaCacheMixin.mediaCacheMaxBytes ~/ 3); + final meta = EncryptedImageUploadResult( + blossomUrl: 'https://blossom/x', + nonce: '00', + mimeType: 'image/png', + originalSize: chunk.length, + width: 1, + height: 1, + filename: 'x.png', + encryptedSize: chunk.length, + ); + + cache.cacheDecryptedImage('a', chunk, meta); + cache.cacheDecryptedImage('b', chunk, meta); + cache.cacheDecryptedImage('c', chunk, meta); + cache.cacheDecryptedImage('d', chunk, meta); + + expect(cache.debugMediaCacheBytes, + lessThanOrEqualTo(MediaCacheMixin.mediaCacheMaxBytes)); + expect(cache.getCachedImage('a'), isNull, + reason: 'oldest entry must be evicted first'); + expect(cache.getCachedImage('d'), isNotNull); + }); + }); +} + +class _CacheHost with MediaCacheMixin {} From e26100f950bbe067fd33938888ac6fee0b2cb358 Mon Sep 17 00:00:00 2001 From: grunch Date: Tue, 1 Sep 2026 15:24:24 -0300 Subject: [PATCH 2/4] fix: narrow the lazy-startup predicate and make the media budget global Addresses PR review feedback: - Startup no longer reuses Status.isTerminal, which answers a different question (may this session be deleted during cleanup?) and is only applied past the expiration cutoff. isSettledOrderMessage covers only statuses after which nothing needing a live reaction arrives, and only once a 24h trailing-notice window has passed. settledHoldInvoice (invoice replacement), canceled (reconcileCanceledBondedSession re-arms the deferred deletion on restart, plus the trailing bond-slashed notice) and success (the unbounded rating exchange) stay eager. Timestamps are unit-normalized and an unknown age counts as live. - The ChatRoomNotifier stays eager for every session with a peer. It is the only consumer of SubscriptionManager.chat, a broadcast stream that drops events with no listener, so a peer message on a finished trade would have been lost until the user opened the Chats tab. - The media budget is now global instead of per notifier, so the ceiling is no longer multiplied by the number of conversations. - Reads promote their entry, making the cache LRU rather than FIFO. - A widget whose bytes were evicted re-requests the image on a cache miss instead of showing the loading placeholder forever. - debugMediaCacheBytes is @visibleForTesting; imports reordered; the guard is dart format clean; tests split to mirror their source modules. --- .../chat/widgets/encrypted_image_message.dart | 6 +- lib/shared/mixins/media_cache_mixin.dart | 103 +++++++++++++----- lib/shared/providers/app_init_provider.dart | 85 +++++++++++---- pubspec.lock | 44 ++++---- .../lazy_terminal_and_media_cache_test.dart | 83 -------------- .../shared/mixins/media_cache_mixin_test.dart | 99 +++++++++++++++++ .../providers/app_init_provider_test.dart | 102 +++++++++++++++++ 7 files changed, 371 insertions(+), 151 deletions(-) delete mode 100644 test/shared/lazy_terminal_and_media_cache_test.dart create mode 100644 test/shared/mixins/media_cache_mixin_test.dart create mode 100644 test/shared/providers/app_init_provider_test.dart diff --git a/lib/features/chat/widgets/encrypted_image_message.dart b/lib/features/chat/widgets/encrypted_image_message.dart index 9d87fe74f..2c8c90149 100644 --- a/lib/features/chat/widgets/encrypted_image_message.dart +++ b/lib/features/chat/widgets/encrypted_image_message.dart @@ -88,7 +88,11 @@ class _EncryptedImageMessageState extends State { return _buildErrorWidget(); } - // Show loading widget while waiting for initState to trigger the load + // Show loading widget while waiting for initState to trigger the load. + // Also covers a cache miss after the media budget evicted the bytes of a + // still-mounted widget: without re-requesting, this would show the + // placeholder forever. + WidgetsBinding.instance.addPostFrameCallback((_) => _loadImageIfNeeded()); return _buildLoadingWidget(); } diff --git a/lib/shared/mixins/media_cache_mixin.dart b/lib/shared/mixins/media_cache_mixin.dart index 9fc1d59fe..eb37d4983 100644 --- a/lib/shared/mixins/media_cache_mixin.dart +++ b/lib/shared/mixins/media_cache_mixin.dart @@ -1,45 +1,90 @@ -import 'dart:typed_data'; +import 'package:flutter/foundation.dart'; import 'package:mostro_mobile/services/encrypted_image_upload_service.dart'; import 'package:mostro_mobile/services/encrypted_file_upload_service.dart'; +/// One cached blob, tracked globally so the budget is a real ceiling rather +/// than a per-conversation one. +class _MediaCacheEntry { + _MediaCacheEntry(this.owner, this.messageId, this.bytes); + + final MediaCacheMixin owner; + final String messageId; + int bytes; +} + /// Shared media cache for decrypted images and files. /// Used by both ChatRoomNotifier (P2P) and DisputeChatNotifier. mixin MediaCacheMixin { - /// Combined byte budget for decrypted media. These notifiers live for the - /// whole app run, so an unbounded cache held every decrypted photo and - /// file forever; the oldest entries are evicted once the budget is hit - /// (they re-decrypt on demand from the stored blob). + /// Combined byte budget for decrypted media, shared by every conversation: + /// these notifiers live for the whole app run, so an unbounded cache held + /// every decrypted photo and file forever, and a per-notifier budget would + /// multiply the ceiling by the number of conversations. The least recently + /// used entries are evicted once the budget is hit (they re-decrypt on + /// demand from the stored blob). static const int mediaCacheMaxBytes = 32 * 1024 * 1024; + static final List<_MediaCacheEntry> _lru = []; + static int _totalBytes = 0; + + @visibleForTesting + static int get debugMediaCacheBytes => _totalBytes; + + @visibleForTesting + static void debugResetMediaCache() { + _lru.clear(); + _totalBytes = 0; + } + final Map _imageCache = {}; final Map _imageMetadata = {}; final Map _fileCache = {}; final Map _fileMetadata = {}; - final List _mediaLru = []; - int _mediaBytes = 0; - - int get debugMediaCacheBytes => _mediaBytes; - - void _mediaTouch(String messageId, int addedBytes) { - _mediaLru.remove(messageId); - _mediaLru.add(messageId); - _mediaBytes += addedBytes; - while (_mediaBytes > mediaCacheMaxBytes && _mediaLru.isNotEmpty) { - final oldest = _mediaLru.removeAt(0); - _mediaBytes -= _imageCache.remove(oldest)?.length ?? 0; - _mediaBytes -= _fileCache.remove(oldest)?.length ?? 0; + + /// Moves an entry to the most-recently-used end. [bytes] is the new size on + /// a write; omitted on a read, which promotes without changing accounting. + void _mediaTouch(String messageId, {int? bytes}) { + final index = _lru.indexWhere( + (e) => identical(e.owner, this) && e.messageId == messageId, + ); + _MediaCacheEntry? entry; + if (index >= 0) { + entry = _lru.removeAt(index); + _totalBytes -= entry.bytes; + } + final size = bytes ?? entry?.bytes; + // A read miss has nothing to promote. + if (size == null) return; + _lru.add(entry == null + ? _MediaCacheEntry(this, messageId, size) + : (entry..bytes = size)); + _totalBytes += size; + _evict(); + } + + static void _evict() { + while (_totalBytes > mediaCacheMaxBytes && _lru.isNotEmpty) { + final oldest = _lru.removeAt(0); + _totalBytes -= oldest.bytes; + // Metadata is kept: it is small, and the widgets re-request the blob on + // a miss, which re-decrypts and re-caches it. + oldest.owner._imageCache.remove(oldest.messageId); + oldest.owner._fileCache.remove(oldest.messageId); } } void cacheDecryptedImage( String messageId, Uint8List data, EncryptedImageUploadResult meta) { - final previous = _imageCache[messageId]?.length ?? 0; _imageCache[messageId] = data; _imageMetadata[messageId] = meta; - _mediaTouch(messageId, data.length - previous); + _mediaTouch(messageId, bytes: data.length); } - Uint8List? getCachedImage(String messageId) => _imageCache[messageId]; + Uint8List? getCachedImage(String messageId) { + final data = _imageCache[messageId]; + // Reads promote too, or the cache would evict a hot entry FIFO-style. + if (data != null) _mediaTouch(messageId); + return data; + } EncryptedImageUploadResult? getImageMetadata(String messageId) => _imageMetadata[messageId]; @@ -47,14 +92,17 @@ mixin MediaCacheMixin { void cacheDecryptedFile( String messageId, Uint8List? data, EncryptedFileUploadResult meta) { if (data != null) { - final previous = _fileCache[messageId]?.length ?? 0; _fileCache[messageId] = data; - _mediaTouch(messageId, data.length - previous); + _mediaTouch(messageId, bytes: data.length); } _fileMetadata[messageId] = meta; } - Uint8List? getCachedFile(String messageId) => _fileCache[messageId]; + Uint8List? getCachedFile(String messageId) { + final data = _fileCache[messageId]; + if (data != null) _mediaTouch(messageId); + return data; + } EncryptedFileUploadResult? getFileMetadata(String messageId) => _fileMetadata[messageId]; @@ -64,7 +112,10 @@ mixin MediaCacheMixin { _imageMetadata.clear(); _fileCache.clear(); _fileMetadata.clear(); - _mediaLru.clear(); - _mediaBytes = 0; + _lru.removeWhere((e) { + if (!identical(e.owner, this)) return false; + _totalBytes -= e.bytes; + return true; + }); } } diff --git a/lib/shared/providers/app_init_provider.dart b/lib/shared/providers/app_init_provider.dart index 16261051f..85d3b5a75 100644 --- a/lib/shared/providers/app_init_provider.dart +++ b/lib/shared/providers/app_init_provider.dart @@ -1,8 +1,9 @@ -import 'package:mostro_mobile/shared/providers/mostro_storage_provider.dart'; -import 'package:mostro_mobile/data/models/order.dart'; -import 'package:mostro_mobile/data/models/mostro_message.dart'; import 'dart:async'; import 'package:flutter_riverpod/flutter_riverpod.dart'; +import 'package:mostro_mobile/data/models/enums/status.dart'; +import 'package:mostro_mobile/data/models/mostro_message.dart'; +import 'package:mostro_mobile/data/models/order.dart'; +import 'package:mostro_mobile/shared/providers/mostro_storage_provider.dart'; import 'package:mostro_mobile/core/config.dart'; import 'package:mostro_mobile/features/key_manager/key_manager_provider.dart'; import 'package:mostro_mobile/features/chat/providers/chat_room_providers.dart'; @@ -38,7 +39,7 @@ final appInitializerProvider = FutureProvider((ref) async { final sessionManager = ref.read(sessionNotifierProvider.notifier); await sessionManager.init(); - + ref.read(subscriptionManagerProvider); // Start the relay health watchdog: re-engages bootstrap relays and @@ -50,7 +51,8 @@ final appInitializerProvider = FutureProvider((ref) async { }); final settings = ref.read(settingsProvider); - final expirationHours = settings.sessionExpirationHours ?? Config.sessionExpirationHours; + final expirationHours = + settings.sessionExpirationHours ?? Config.sessionExpirationHours; final isForever = expirationHours == 0; final cutoff = isForever ? null @@ -58,27 +60,72 @@ final appInitializerProvider = FutureProvider((ref) async { final messageStorage = ref.read(mostroStorageProvider); for (final session in sessionManager.sessions) { - if(session.orderId == null || (cutoff != null && session.startTime.isBefore(cutoff))) continue; - - // Terminal orders initialize lazily when a screen watches them: an eager - // notifier per finished trade meant a storage watcher, a book listener - // and (for chats) a history decrypt alive until process exit. - final latest = - await messageStorage.getLatestMessageById(session.orderId!); - if (isTerminalOrderMessage(latest)) continue; + if (session.orderId == null || + (cutoff != null && session.startTime.isBefore(cutoff))) { + continue; + } - ref.read(orderNotifierProvider(session.orderId!).notifier); + // Settled orders initialize lazily when a screen watches them: an eager + // notifier per finished trade meant a storage watcher and a book listener + // alive until process exit. + final latest = await messageStorage.getLatestMessageById(session.orderId!); + if (!isSettledOrderMessage(latest)) { + ref.read(orderNotifierProvider(session.orderId!).notifier); + } + // The chat notifier stays eager regardless: it is the only consumer of + // SubscriptionManager.chat, a broadcast stream that drops events while + // nobody listens, so a peer message on a finished trade would be lost + // until the user happened to open the Chats tab. if (session.peer != null) { ref.read(chatRoomsProvider(session.orderId!)); } } }); -/// Whether the order's last stored message reports a terminal status. A -/// missing message or a non-order payload counts as live, so anything -/// ambiguous keeps today's eager behaviour. -bool isTerminalOrderMessage(MostroMessage? message) { +/// How long after the last message a finished order still initializes +/// eagerly, so trailing notices (`bond-slashed`, ratings) are reacted to +/// live rather than only persisted. +const Duration settledOrderGrace = Duration(hours: 24); + +/// Statuses after which Mostro sends nothing that needs a live reaction. +/// +/// Deliberately *not* [Status.isTerminal], which answers a different +/// question — whether a session can be deleted during cleanup — and is only +/// ever applied to sessions already past the expiration cutoff. Three of its +/// members still expect traffic and stay eager here: +/// * [Status.settledHoldInvoice] — the window between release and payout, +/// where the buyer may still replace a wrong invoice; +/// * [Status.canceled] — carries the deferred session deletion that +/// `OrderNotifier.sync()` re-arms via `reconcileCanceledBondedSession()`, +/// plus a trailing `bond-slashed` notice; +/// * [Status.success] — the rating exchange, which has no time bound. +const Set settledOrderStatuses = { + Status.canceledByAdmin, + Status.settledByAdmin, + Status.completedByAdmin, + Status.cooperativelyCanceled, + Status.expired, +}; + +/// Whether the order's last stored message reports a settled status old +/// enough that nothing further is expected. A missing message, a non-order +/// payload, an unknown timestamp or a recent one all count as live, so +/// anything ambiguous keeps today's eager behaviour. +bool isSettledOrderMessage(MostroMessage? message, {DateTime? now}) { final order = message?.getPayload(); - return order != null && order.status.isTerminal; + if (order == null || !settledOrderStatuses.contains(order.status)) { + return false; + } + final at = _messageTime(message!.timestamp); + if (at == null) return false; + return (now ?? DateTime.now()).difference(at) > settledOrderGrace; +} + +/// The daemon sends seconds, the app fills in milliseconds when the field is +/// absent, and both units coexist in the store. +DateTime? _messageTime(int? raw) { + if (raw == null || raw <= 0) return null; + final ms = raw < 1000000000000 ? raw * 1000 : raw; + return DateTime.fromMillisecondsSinceEpoch(ms); } diff --git a/pubspec.lock b/pubspec.lock index 9c358b502..41dda05ba 100644 --- a/pubspec.lock +++ b/pubspec.lock @@ -5,10 +5,10 @@ packages: dependency: transitive description: name: _fe_analyzer_shared - sha256: da0d9209ca76bde579f2da330aeb9df62b6319c834fa7baae052021b0462401f + sha256: c209688d9f5a5f26b2fb47a188131a6fb9e876ae9e47af3737c0b4f58a93470d url: "https://pub.dev" source: hosted - version: "85.0.0" + version: "91.0.0" _flutterfire_internals: dependency: transitive description: @@ -21,10 +21,10 @@ packages: dependency: transitive description: name: analyzer - sha256: "974859dc0ff5f37bc4313244b3218c791810d03ab3470a579580279ba971a48d" + sha256: f51c8499b35f9b26820cfe914828a6a98a94efd5cc78b37bb7d03debae3a1d08 url: "https://pub.dev" source: hosted - version: "7.7.1" + version: "8.4.1" app_links: dependency: "direct main" description: @@ -221,10 +221,10 @@ packages: dependency: transitive description: name: characters - sha256: f71061c654a3380576a52b451dd5532377954cf9dbd272a78fc8479606670803 + sha256: faf38497bda5ead2a8c7615f4f7939df04333478bf32e4173fcb06d428b5716b url: "https://pub.dev" source: hosted - version: "1.4.0" + version: "1.4.1" checked_yaml: dependency: transitive description: @@ -342,10 +342,10 @@ packages: dependency: transitive description: name: dart_style - sha256: "8a0e5fba27e8ee025d2ffb4ee820b4e6e2cf5e4246a6b1a477eb66866947e0bb" + sha256: a9c30492da18ff84efe2422ba2d319a89942d93e58eb0b73d32abe822ef54b7b url: "https://pub.dev" source: hosted - version: "3.1.1" + version: "3.1.3" dbus: dependency: transitive description: @@ -1073,26 +1073,26 @@ packages: dependency: transitive description: name: matcher - sha256: dc58c723c3c24bf8d3e2d3ad3f2f9d7bd9cf43ec6feaa64181775e60190153f2 + sha256: dc0b7dc7651697ea4ff3e69ef44b0407ea32c487a39fff6a4004fa585e901861 url: "https://pub.dev" source: hosted - version: "0.12.17" + version: "0.12.19" material_color_utilities: dependency: transitive description: name: material_color_utilities - sha256: f7142bb1154231d7ea5f96bc7bde4bda2a0945d2806bb11670e30b850d56bdec + sha256: "9c337007e82b1889149c82ed242ed1cb24a66044e30979c44912381e9be4c48b" url: "https://pub.dev" source: hosted - version: "0.11.1" + version: "0.13.0" meta: dependency: transitive description: name: meta - sha256: e3641ec5d63ebf0d9b41bd43201a66e3fc79a65db5f61fc181f04cd27aab950c + sha256: "23f08335362185a5ea2ad3a4e597f1375e78bce8a040df5c600c8d3552ef2394" url: "https://pub.dev" source: hosted - version: "1.16.0" + version: "1.17.0" mime: dependency: "direct main" description: @@ -1113,10 +1113,10 @@ packages: dependency: "direct dev" description: name: mockito - sha256: "2314cbe9165bcd16106513df9cf3c3224713087f09723b128928dc11a4379f99" + sha256: eff30d002f0c8bf073b6f929df4483b543133fcafce056870163587b03f1d422 url: "https://pub.dev" source: hosted - version: "5.5.0" + version: "5.6.4" mutation_test: dependency: "direct dev" description: @@ -1655,26 +1655,26 @@ packages: dependency: "direct dev" description: name: test - sha256: "65e29d831719be0591f7b3b1a32a3cda258ec98c58c7b25f7b84241bc31215bb" + sha256: "280d6d890011ca966ad08df7e8a4ddfab0fb3aa49f96ed6de56e3521347a9ae7" url: "https://pub.dev" source: hosted - version: "1.26.2" + version: "1.30.0" test_api: dependency: transitive description: name: test_api - sha256: "522f00f556e73044315fa4585ec3270f1808a4b186c936e612cab0b565ff1e00" + sha256: "8161c84903fd860b26bfdefb7963b3f0b68fee7adea0f59ef805ecca346f0c7a" url: "https://pub.dev" source: hosted - version: "0.7.6" + version: "0.7.10" test_core: dependency: transitive description: name: test_core - sha256: "80bf5a02b60af04b09e14f6fe68b921aad119493e26e490deaca5993fef1b05a" + sha256: "0381bd1585d1a924763c308100f2138205252fb90c9d4eeaf28489ee65ccde51" url: "https://pub.dev" source: hosted - version: "0.6.11" + version: "0.6.16" timeago: dependency: "direct main" description: diff --git a/test/shared/lazy_terminal_and_media_cache_test.dart b/test/shared/lazy_terminal_and_media_cache_test.dart deleted file mode 100644 index 55fad4e69..000000000 --- a/test/shared/lazy_terminal_and_media_cache_test.dart +++ /dev/null @@ -1,83 +0,0 @@ -import 'dart:typed_data'; - -import 'package:flutter_test/flutter_test.dart'; -import 'package:mostro_mobile/data/models/enums/action.dart'; -import 'package:mostro_mobile/data/models/enums/order_type.dart'; -import 'package:mostro_mobile/data/models/enums/status.dart'; -import 'package:mostro_mobile/data/models/mostro_message.dart'; -import 'package:mostro_mobile/data/models/order.dart'; -import 'package:mostro_mobile/services/encrypted_image_upload_service.dart'; -import 'package:mostro_mobile/shared/mixins/media_cache_mixin.dart'; -import 'package:mostro_mobile/shared/providers/app_init_provider.dart'; - -/// Startup eagerly created an OrderNotifier (storage watcher + listeners) and -/// a ChatRoomNotifier (history decrypt) for EVERY session of the last 30 -/// days, terminal or not, and each chat notifier held its decrypted media -/// bytes until process exit. Terminal orders now initialize lazily and the -/// media cache is byte-bounded. -void main() { - MostroMessage withStatus(Status status) => MostroMessage( - action: Action.newOrder, - id: 'o1', - payload: Order( - kind: OrderType.sell, - status: status, - fiatCode: 'VES', - fiatAmount: 100, - paymentMethod: 'cash', - ), - ); - - group('isTerminalOrderMessage', () { - test('terminal statuses skip eager initialization', () { - expect(isTerminalOrderMessage(withStatus(Status.canceled)), isTrue); - expect(isTerminalOrderMessage(withStatus(Status.success)), isTrue); - expect( - isTerminalOrderMessage(withStatus(Status.canceledByAdmin)), isTrue); - }); - - test('live statuses keep eager initialization', () { - expect(isTerminalOrderMessage(withStatus(Status.pending)), isFalse); - expect(isTerminalOrderMessage(withStatus(Status.active)), isFalse); - expect(isTerminalOrderMessage(withStatus(Status.fiatSent)), isFalse); - }); - - test('no message or no order payload counts as live (conservative)', () { - expect(isTerminalOrderMessage(null), isFalse); - expect( - isTerminalOrderMessage(MostroMessage(action: Action.rate, id: 'o1')), - isFalse, - ); - }); - }); - - group('MediaCacheMixin byte bound', () { - test('evicts the oldest entries once the cap is exceeded', () { - final cache = _CacheHost(); - final chunk = Uint8List(MediaCacheMixin.mediaCacheMaxBytes ~/ 3); - final meta = EncryptedImageUploadResult( - blossomUrl: 'https://blossom/x', - nonce: '00', - mimeType: 'image/png', - originalSize: chunk.length, - width: 1, - height: 1, - filename: 'x.png', - encryptedSize: chunk.length, - ); - - cache.cacheDecryptedImage('a', chunk, meta); - cache.cacheDecryptedImage('b', chunk, meta); - cache.cacheDecryptedImage('c', chunk, meta); - cache.cacheDecryptedImage('d', chunk, meta); - - expect(cache.debugMediaCacheBytes, - lessThanOrEqualTo(MediaCacheMixin.mediaCacheMaxBytes)); - expect(cache.getCachedImage('a'), isNull, - reason: 'oldest entry must be evicted first'); - expect(cache.getCachedImage('d'), isNotNull); - }); - }); -} - -class _CacheHost with MediaCacheMixin {} diff --git a/test/shared/mixins/media_cache_mixin_test.dart b/test/shared/mixins/media_cache_mixin_test.dart new file mode 100644 index 000000000..211fee3f0 --- /dev/null +++ b/test/shared/mixins/media_cache_mixin_test.dart @@ -0,0 +1,99 @@ +import 'dart:typed_data'; + +import 'package:flutter_test/flutter_test.dart'; +import 'package:mostro_mobile/services/encrypted_image_upload_service.dart'; +import 'package:mostro_mobile/shared/mixins/media_cache_mixin.dart'; + +/// Every chat notifier held its decrypted media bytes until process exit. The +/// cache is now byte-bounded, and the budget is global: a per-notifier +/// ceiling would have been multiplied by the number of conversations. +void main() { + EncryptedImageUploadResult meta(int size) => EncryptedImageUploadResult( + blossomUrl: 'https://blossom/x', + nonce: '00', + mimeType: 'image/png', + originalSize: size, + width: 1, + height: 1, + filename: 'x.png', + encryptedSize: size, + ); + + Uint8List chunk() => Uint8List(MediaCacheMixin.mediaCacheMaxBytes ~/ 3); + + setUp(MediaCacheMixin.debugResetMediaCache); + + test('evicts the least recently used entries once the cap is exceeded', () { + final cache = _CacheHost(); + final data = chunk(); + + cache.cacheDecryptedImage('a', data, meta(data.length)); + cache.cacheDecryptedImage('b', data, meta(data.length)); + cache.cacheDecryptedImage('c', data, meta(data.length)); + cache.cacheDecryptedImage('d', data, meta(data.length)); + + expect(MediaCacheMixin.debugMediaCacheBytes, + lessThanOrEqualTo(MediaCacheMixin.mediaCacheMaxBytes)); + expect(cache.getCachedImage('a'), isNull, + reason: 'least recently used entry must be evicted first'); + expect(cache.getCachedImage('d'), isNotNull); + }); + + test('a read promotes an entry, so a hot item is not evicted', () { + final cache = _CacheHost(); + final data = chunk(); + + cache.cacheDecryptedImage('a', data, meta(data.length)); + cache.cacheDecryptedImage('b', data, meta(data.length)); + cache.getCachedImage('a'); + cache.cacheDecryptedImage('c', data, meta(data.length)); + cache.cacheDecryptedImage('d', data, meta(data.length)); + + expect(cache.getCachedImage('a'), isNotNull, + reason: 'read recently: FIFO would have evicted it'); + expect(cache.getCachedImage('b'), isNull); + }); + + test('a read does not change the byte accounting', () { + final cache = _CacheHost(); + final data = chunk(); + + cache.cacheDecryptedImage('a', data, meta(data.length)); + final before = MediaCacheMixin.debugMediaCacheBytes; + cache.getCachedImage('a'); + cache.getCachedImage('missing'); + + expect(MediaCacheMixin.debugMediaCacheBytes, before); + }); + + test('the budget is shared across conversations, not per notifier', () { + final first = _CacheHost(); + final second = _CacheHost(); + final data = chunk(); + + first.cacheDecryptedImage('a', data, meta(data.length)); + first.cacheDecryptedImage('b', data, meta(data.length)); + second.cacheDecryptedImage('c', data, meta(data.length)); + second.cacheDecryptedImage('d', data, meta(data.length)); + + expect(MediaCacheMixin.debugMediaCacheBytes, + lessThanOrEqualTo(MediaCacheMixin.mediaCacheMaxBytes)); + expect(first.getCachedImage('a'), isNull, + reason: 'a second conversation must not double the ceiling'); + }); + + test('clearing one conversation releases only its own bytes', () { + final first = _CacheHost(); + final second = _CacheHost(); + final data = chunk(); + + first.cacheDecryptedImage('a', data, meta(data.length)); + second.cacheDecryptedImage('b', data, meta(data.length)); + first.clearMediaCaches(); + + expect(MediaCacheMixin.debugMediaCacheBytes, data.length); + expect(second.getCachedImage('b'), isNotNull); + }); +} + +class _CacheHost with MediaCacheMixin {} diff --git a/test/shared/providers/app_init_provider_test.dart b/test/shared/providers/app_init_provider_test.dart new file mode 100644 index 000000000..dae3ecb8e --- /dev/null +++ b/test/shared/providers/app_init_provider_test.dart @@ -0,0 +1,102 @@ +import 'package:flutter_test/flutter_test.dart'; +import 'package:mostro_mobile/data/models/enums/action.dart'; +import 'package:mostro_mobile/data/models/enums/order_type.dart'; +import 'package:mostro_mobile/data/models/enums/status.dart'; +import 'package:mostro_mobile/data/models/mostro_message.dart'; +import 'package:mostro_mobile/data/models/order.dart'; +import 'package:mostro_mobile/shared/providers/app_init_provider.dart'; + +/// Startup eagerly created an OrderNotifier (storage watcher + book listener) +/// for EVERY session of the last 30 days, settled or not. Settled orders now +/// initialize lazily — but only those after which Mostro sends nothing that +/// needs a live reaction, and only once the trailing-notice window has +/// passed. `Status.isTerminal` answers a different question (may this session +/// be deleted during cleanup?) and is not reused here. +void main() { + final now = DateTime(2026, 9, 1, 12); + + MostroMessage withStatus(Status status, + {Duration age = const Duration(days: 3)}) { + final message = MostroMessage( + action: Action.newOrder, + id: 'o1', + payload: Order( + kind: OrderType.sell, + status: status, + fiatCode: 'VES', + fiatAmount: 100, + paymentMethod: 'cash', + ), + ); + message.timestamp = now.subtract(age).millisecondsSinceEpoch; + return message; + } + + group('isSettledOrderMessage', () { + test('settled statuses past the grace window initialize lazily', () { + for (final status in settledOrderStatuses) { + expect(isSettledOrderMessage(withStatus(status), now: now), isTrue, + reason: '$status expects no further traffic'); + } + }); + + test('a settled order still inside the grace window stays eager', () { + // Trailing notices (bond-slashed, ratings) must still be reacted to + // live, not only persisted. + expect( + isSettledOrderMessage( + withStatus(Status.canceledByAdmin, age: const Duration(hours: 1)), + now: now), + isFalse, + ); + }); + + test('statuses that still expect traffic stay eager', () { + // settled-hold-invoice: the buyer may still replace a wrong invoice. + expect( + isSettledOrderMessage(withStatus(Status.settledHoldInvoice), + now: now), + isFalse); + // canceled: OrderNotifier.sync() re-arms the deferred session deletion + // through reconcileCanceledBondedSession(), and bond-slashed trails it. + expect(isSettledOrderMessage(withStatus(Status.canceled), now: now), + isFalse); + // success: the rating exchange has no time bound. + expect( + isSettledOrderMessage(withStatus(Status.success), now: now), isFalse); + }); + + test('live statuses keep eager initialization', () { + expect( + isSettledOrderMessage(withStatus(Status.pending), now: now), isFalse); + expect( + isSettledOrderMessage(withStatus(Status.active), now: now), isFalse); + expect(isSettledOrderMessage(withStatus(Status.fiatSent), now: now), + isFalse); + }); + + test('anything ambiguous counts as live (conservative)', () { + expect(isSettledOrderMessage(null, now: now), isFalse); + expect( + isSettledOrderMessage(MostroMessage(action: Action.rate, id: 'o1'), + now: now), + isFalse, + reason: 'no order payload', + ); + final noTimestamp = withStatus(Status.expired); + noTimestamp.timestamp = null; + expect(isSettledOrderMessage(noTimestamp, now: now), isFalse, + reason: 'an unknown age is not proof that nothing is coming'); + }); + + test('a seconds timestamp is read in the right unit', () { + // The daemon sends seconds; the app fills in milliseconds only when the + // field is absent, so both units coexist in the store. + final message = withStatus(Status.expired); + message.timestamp = + now.subtract(const Duration(hours: 1)).millisecondsSinceEpoch ~/ 1000; + expect(isSettledOrderMessage(message, now: now), isFalse, + reason: 'one hour old: still inside the grace window'); + }); + }); +} From a06a9c6e0e3ee9009e23bb72c3f5e5ae777f2182 Mon Sep 17 00:00:00 2001 From: grunch Date: Wed, 2 Sep 2026 18:01:09 -0300 Subject: [PATCH 3/4] fix: never evict the media entry just inserted and drop the stray lock bump An entry larger than the whole budget evicted itself, and the post-frame re-request in EncryptedImageMessage turned that into an endless download-and-decrypt loop. Eviction now stops at the most recent entry. Image and file bytes under the same message id are accounted separately, the startup storage lookups are issued together instead of awaited one by one, and pubspec.lock is restored to main (the analyzer 7 -> 8 bump was unrelated to this PR). --- lib/shared/mixins/media_cache_mixin.dart | 39 +++++++++---- lib/shared/providers/app_init_provider.dart | 21 ++++--- pubspec.lock | 44 +++++++-------- .../shared/mixins/media_cache_mixin_test.dart | 55 +++++++++++++++++++ 4 files changed, 119 insertions(+), 40 deletions(-) diff --git a/lib/shared/mixins/media_cache_mixin.dart b/lib/shared/mixins/media_cache_mixin.dart index eb37d4983..01301a627 100644 --- a/lib/shared/mixins/media_cache_mixin.dart +++ b/lib/shared/mixins/media_cache_mixin.dart @@ -5,13 +5,19 @@ import 'package:mostro_mobile/services/encrypted_file_upload_service.dart'; /// One cached blob, tracked globally so the budget is a real ceiling rather /// than a per-conversation one. class _MediaCacheEntry { - _MediaCacheEntry(this.owner, this.messageId, this.bytes); + _MediaCacheEntry(this.owner, this.kind, this.messageId, this.bytes); final MediaCacheMixin owner; + final _MediaKind kind; final String messageId; int bytes; } +/// Images and files are separate maps, so one message id may hold both; +/// keying the accounting by kind keeps their bytes from overwriting each +/// other. +enum _MediaKind { image, file } + /// Shared media cache for decrypted images and files. /// Used by both ChatRoomNotifier (P2P) and DisputeChatNotifier. mixin MediaCacheMixin { @@ -42,9 +48,12 @@ mixin MediaCacheMixin { /// Moves an entry to the most-recently-used end. [bytes] is the new size on /// a write; omitted on a read, which promotes without changing accounting. - void _mediaTouch(String messageId, {int? bytes}) { + void _mediaTouch(_MediaKind kind, String messageId, {int? bytes}) { final index = _lru.indexWhere( - (e) => identical(e.owner, this) && e.messageId == messageId, + (e) => + identical(e.owner, this) && + e.kind == kind && + e.messageId == messageId, ); _MediaCacheEntry? entry; if (index >= 0) { @@ -55,20 +64,28 @@ mixin MediaCacheMixin { // A read miss has nothing to promote. if (size == null) return; _lru.add(entry == null - ? _MediaCacheEntry(this, messageId, size) + ? _MediaCacheEntry(this, kind, messageId, size) : (entry..bytes = size)); _totalBytes += size; _evict(); } + /// Evicts from the least recently used end until the budget is met. The + /// entry just touched (the last one) is never evicted: an entry larger than + /// the whole budget would otherwise evict itself, and the widget that asked + /// for it would re-download and re-decrypt it on every rebuild. static void _evict() { - while (_totalBytes > mediaCacheMaxBytes && _lru.isNotEmpty) { + while (_totalBytes > mediaCacheMaxBytes && _lru.length > 1) { final oldest = _lru.removeAt(0); _totalBytes -= oldest.bytes; // Metadata is kept: it is small, and the widgets re-request the blob on // a miss, which re-decrypts and re-caches it. - oldest.owner._imageCache.remove(oldest.messageId); - oldest.owner._fileCache.remove(oldest.messageId); + switch (oldest.kind) { + case _MediaKind.image: + oldest.owner._imageCache.remove(oldest.messageId); + case _MediaKind.file: + oldest.owner._fileCache.remove(oldest.messageId); + } } } @@ -76,13 +93,13 @@ mixin MediaCacheMixin { String messageId, Uint8List data, EncryptedImageUploadResult meta) { _imageCache[messageId] = data; _imageMetadata[messageId] = meta; - _mediaTouch(messageId, bytes: data.length); + _mediaTouch(_MediaKind.image, messageId, bytes: data.length); } Uint8List? getCachedImage(String messageId) { final data = _imageCache[messageId]; // Reads promote too, or the cache would evict a hot entry FIFO-style. - if (data != null) _mediaTouch(messageId); + if (data != null) _mediaTouch(_MediaKind.image, messageId); return data; } @@ -93,14 +110,14 @@ mixin MediaCacheMixin { String messageId, Uint8List? data, EncryptedFileUploadResult meta) { if (data != null) { _fileCache[messageId] = data; - _mediaTouch(messageId, bytes: data.length); + _mediaTouch(_MediaKind.file, messageId, bytes: data.length); } _fileMetadata[messageId] = meta; } Uint8List? getCachedFile(String messageId) { final data = _fileCache[messageId]; - if (data != null) _mediaTouch(messageId); + if (data != null) _mediaTouch(_MediaKind.file, messageId); return data; } diff --git a/lib/shared/providers/app_init_provider.dart b/lib/shared/providers/app_init_provider.dart index 85d3b5a75..4b5aa370a 100644 --- a/lib/shared/providers/app_init_provider.dart +++ b/lib/shared/providers/app_init_provider.dart @@ -59,17 +59,24 @@ final appInitializerProvider = FutureProvider((ref) async { : DateTime.now().subtract(Duration(hours: expirationHours)); final messageStorage = ref.read(mostroStorageProvider); - for (final session in sessionManager.sessions) { - if (session.orderId == null || - (cutoff != null && session.startTime.isBefore(cutoff))) { - continue; - } + final sessions = sessionManager.sessions + .where((session) => + session.orderId != null && + (cutoff == null || !session.startTime.isBefore(cutoff))) + .toList(); + // One storage lookup per session, issued together rather than awaited one + // by one, so the added startup cost is a single round trip. + final latestMessages = await Future.wait( + sessions.map((s) => messageStorage.getLatestMessageById(s.orderId!)), + ); + + for (var i = 0; i < sessions.length; i++) { + final session = sessions[i]; // Settled orders initialize lazily when a screen watches them: an eager // notifier per finished trade meant a storage watcher and a book listener // alive until process exit. - final latest = await messageStorage.getLatestMessageById(session.orderId!); - if (!isSettledOrderMessage(latest)) { + if (!isSettledOrderMessage(latestMessages[i])) { ref.read(orderNotifierProvider(session.orderId!).notifier); } diff --git a/pubspec.lock b/pubspec.lock index 41dda05ba..9c358b502 100644 --- a/pubspec.lock +++ b/pubspec.lock @@ -5,10 +5,10 @@ packages: dependency: transitive description: name: _fe_analyzer_shared - sha256: c209688d9f5a5f26b2fb47a188131a6fb9e876ae9e47af3737c0b4f58a93470d + sha256: da0d9209ca76bde579f2da330aeb9df62b6319c834fa7baae052021b0462401f url: "https://pub.dev" source: hosted - version: "91.0.0" + version: "85.0.0" _flutterfire_internals: dependency: transitive description: @@ -21,10 +21,10 @@ packages: dependency: transitive description: name: analyzer - sha256: f51c8499b35f9b26820cfe914828a6a98a94efd5cc78b37bb7d03debae3a1d08 + sha256: "974859dc0ff5f37bc4313244b3218c791810d03ab3470a579580279ba971a48d" url: "https://pub.dev" source: hosted - version: "8.4.1" + version: "7.7.1" app_links: dependency: "direct main" description: @@ -221,10 +221,10 @@ packages: dependency: transitive description: name: characters - sha256: faf38497bda5ead2a8c7615f4f7939df04333478bf32e4173fcb06d428b5716b + sha256: f71061c654a3380576a52b451dd5532377954cf9dbd272a78fc8479606670803 url: "https://pub.dev" source: hosted - version: "1.4.1" + version: "1.4.0" checked_yaml: dependency: transitive description: @@ -342,10 +342,10 @@ packages: dependency: transitive description: name: dart_style - sha256: a9c30492da18ff84efe2422ba2d319a89942d93e58eb0b73d32abe822ef54b7b + sha256: "8a0e5fba27e8ee025d2ffb4ee820b4e6e2cf5e4246a6b1a477eb66866947e0bb" url: "https://pub.dev" source: hosted - version: "3.1.3" + version: "3.1.1" dbus: dependency: transitive description: @@ -1073,26 +1073,26 @@ packages: dependency: transitive description: name: matcher - sha256: dc0b7dc7651697ea4ff3e69ef44b0407ea32c487a39fff6a4004fa585e901861 + sha256: dc58c723c3c24bf8d3e2d3ad3f2f9d7bd9cf43ec6feaa64181775e60190153f2 url: "https://pub.dev" source: hosted - version: "0.12.19" + version: "0.12.17" material_color_utilities: dependency: transitive description: name: material_color_utilities - sha256: "9c337007e82b1889149c82ed242ed1cb24a66044e30979c44912381e9be4c48b" + sha256: f7142bb1154231d7ea5f96bc7bde4bda2a0945d2806bb11670e30b850d56bdec url: "https://pub.dev" source: hosted - version: "0.13.0" + version: "0.11.1" meta: dependency: transitive description: name: meta - sha256: "23f08335362185a5ea2ad3a4e597f1375e78bce8a040df5c600c8d3552ef2394" + sha256: e3641ec5d63ebf0d9b41bd43201a66e3fc79a65db5f61fc181f04cd27aab950c url: "https://pub.dev" source: hosted - version: "1.17.0" + version: "1.16.0" mime: dependency: "direct main" description: @@ -1113,10 +1113,10 @@ packages: dependency: "direct dev" description: name: mockito - sha256: eff30d002f0c8bf073b6f929df4483b543133fcafce056870163587b03f1d422 + sha256: "2314cbe9165bcd16106513df9cf3c3224713087f09723b128928dc11a4379f99" url: "https://pub.dev" source: hosted - version: "5.6.4" + version: "5.5.0" mutation_test: dependency: "direct dev" description: @@ -1655,26 +1655,26 @@ packages: dependency: "direct dev" description: name: test - sha256: "280d6d890011ca966ad08df7e8a4ddfab0fb3aa49f96ed6de56e3521347a9ae7" + sha256: "65e29d831719be0591f7b3b1a32a3cda258ec98c58c7b25f7b84241bc31215bb" url: "https://pub.dev" source: hosted - version: "1.30.0" + version: "1.26.2" test_api: dependency: transitive description: name: test_api - sha256: "8161c84903fd860b26bfdefb7963b3f0b68fee7adea0f59ef805ecca346f0c7a" + sha256: "522f00f556e73044315fa4585ec3270f1808a4b186c936e612cab0b565ff1e00" url: "https://pub.dev" source: hosted - version: "0.7.10" + version: "0.7.6" test_core: dependency: transitive description: name: test_core - sha256: "0381bd1585d1a924763c308100f2138205252fb90c9d4eeaf28489ee65ccde51" + sha256: "80bf5a02b60af04b09e14f6fe68b921aad119493e26e490deaca5993fef1b05a" url: "https://pub.dev" source: hosted - version: "0.6.16" + version: "0.6.11" timeago: dependency: "direct main" description: diff --git a/test/shared/mixins/media_cache_mixin_test.dart b/test/shared/mixins/media_cache_mixin_test.dart index 211fee3f0..7be84455d 100644 --- a/test/shared/mixins/media_cache_mixin_test.dart +++ b/test/shared/mixins/media_cache_mixin_test.dart @@ -1,6 +1,7 @@ import 'dart:typed_data'; import 'package:flutter_test/flutter_test.dart'; +import 'package:mostro_mobile/services/encrypted_file_upload_service.dart'; import 'package:mostro_mobile/services/encrypted_image_upload_service.dart'; import 'package:mostro_mobile/shared/mixins/media_cache_mixin.dart'; @@ -19,6 +20,16 @@ void main() { encryptedSize: size, ); + EncryptedFileUploadResult fileMeta(int size) => EncryptedFileUploadResult( + blossomUrl: 'https://blossom/f', + nonce: '00', + mimeType: 'application/pdf', + fileType: 'pdf', + originalSize: size, + filename: 'f.pdf', + encryptedSize: size, + ); + Uint8List chunk() => Uint8List(MediaCacheMixin.mediaCacheMaxBytes ~/ 3); setUp(MediaCacheMixin.debugResetMediaCache); @@ -82,6 +93,50 @@ void main() { reason: 'a second conversation must not double the ceiling'); }); + test( + 'an entry larger than the budget stays cached instead of evicting itself', + () { + // Evicting the entry just inserted made the image widget re-download and + // re-decrypt on every frame: cache miss -> load -> cache -> self-evict -> + // setState -> cache miss again. + final cache = _CacheHost(); + final huge = Uint8List(MediaCacheMixin.mediaCacheMaxBytes + 1); + + cache.cacheDecryptedImage('huge', huge, meta(huge.length)); + + expect(cache.getCachedImage('huge'), isNotNull); + expect(MediaCacheMixin.debugMediaCacheBytes, huge.length); + }); + + test('an oversized entry still evicts everything older than itself', () { + final cache = _CacheHost(); + final data = chunk(); + final huge = Uint8List(MediaCacheMixin.mediaCacheMaxBytes + 1); + + cache.cacheDecryptedImage('a', data, meta(data.length)); + cache.cacheDecryptedImage('huge', huge, meta(huge.length)); + + expect(cache.getCachedImage('a'), isNull); + expect(MediaCacheMixin.debugMediaCacheBytes, huge.length); + }); + + test('an image and a file under the same message id are tracked separately', + () { + final cache = _CacheHost(); + final image = Uint8List(4); + final file = Uint8List(1); + + cache.cacheDecryptedImage('m', image, meta(image.length)); + cache.cacheDecryptedFile('m', file, fileMeta(file.length)); + + expect(MediaCacheMixin.debugMediaCacheBytes, image.length + file.length); + expect(cache.getCachedImage('m'), isNotNull); + expect(cache.getCachedFile('m'), isNotNull); + + cache.clearMediaCaches(); + expect(MediaCacheMixin.debugMediaCacheBytes, 0); + }); + test('clearing one conversation releases only its own bytes', () { final first = _CacheHost(); final second = _CacheHost(); From b0d2052838b19fd7887db12cb6cfbdb8f7ef565a Mon Sep 17 00:00:00 2001 From: grunch Date: Thu, 3 Sep 2026 11:06:17 -0300 Subject: [PATCH 4/4] fix: reject out-of-range timestamps and hold media cache owners weakly A stored message timestamp beyond the DateTime range made `_messageTime` throw a RangeError from `isSettledOrderMessage`, which aborted app initialization instead of treating the message as live. Values past 8640000000000000 ms now count as an unknown age. `_MediaCacheEntry.owner` was a strong reference held from a process-global static list, so a notifier whose `dispose()` never ran would have been kept alive together with every decrypted blob in its maps. The entry now holds a `WeakReference`; eviction skips an already collected owner and only drops its accounting. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01BDpY7XJcGHDtN21KUJA8e4 --- lib/shared/mixins/media_cache_mixin.dart | 22 ++++++++++++------- lib/shared/providers/app_init_provider.dart | 9 +++++++- .../providers/app_init_provider_test.dart | 9 ++++++++ 3 files changed, 31 insertions(+), 9 deletions(-) diff --git a/lib/shared/mixins/media_cache_mixin.dart b/lib/shared/mixins/media_cache_mixin.dart index 01301a627..2b71ca1e9 100644 --- a/lib/shared/mixins/media_cache_mixin.dart +++ b/lib/shared/mixins/media_cache_mixin.dart @@ -3,11 +3,14 @@ import 'package:mostro_mobile/services/encrypted_image_upload_service.dart'; import 'package:mostro_mobile/services/encrypted_file_upload_service.dart'; /// One cached blob, tracked globally so the budget is a real ceiling rather -/// than a per-conversation one. +/// than a per-conversation one. The owner is held weakly: the list is +/// process-global, and a strong reference would keep a notifier (and every +/// blob in its maps) alive if `dispose()` never ran. class _MediaCacheEntry { - _MediaCacheEntry(this.owner, this.kind, this.messageId, this.bytes); + _MediaCacheEntry(MediaCacheMixin owner, this.kind, this.messageId, this.bytes) + : owner = WeakReference(owner); - final MediaCacheMixin owner; + final WeakReference owner; final _MediaKind kind; final String messageId; int bytes; @@ -51,7 +54,7 @@ mixin MediaCacheMixin { void _mediaTouch(_MediaKind kind, String messageId, {int? bytes}) { final index = _lru.indexWhere( (e) => - identical(e.owner, this) && + identical(e.owner.target, this) && e.kind == kind && e.messageId == messageId, ); @@ -79,12 +82,15 @@ mixin MediaCacheMixin { final oldest = _lru.removeAt(0); _totalBytes -= oldest.bytes; // Metadata is kept: it is small, and the widgets re-request the blob on - // a miss, which re-decrypts and re-caches it. + // a miss, which re-decrypts and re-caches it. A collected owner has + // already released its maps; only the accounting was left to drop. + final owner = oldest.owner.target; + if (owner == null) continue; switch (oldest.kind) { case _MediaKind.image: - oldest.owner._imageCache.remove(oldest.messageId); + owner._imageCache.remove(oldest.messageId); case _MediaKind.file: - oldest.owner._fileCache.remove(oldest.messageId); + owner._fileCache.remove(oldest.messageId); } } } @@ -130,7 +136,7 @@ mixin MediaCacheMixin { _fileCache.clear(); _fileMetadata.clear(); _lru.removeWhere((e) { - if (!identical(e.owner, this)) return false; + if (!identical(e.owner.target, this)) return false; _totalBytes -= e.bytes; return true; }); diff --git a/lib/shared/providers/app_init_provider.dart b/lib/shared/providers/app_init_provider.dart index 4b5aa370a..2ddda940a 100644 --- a/lib/shared/providers/app_init_provider.dart +++ b/lib/shared/providers/app_init_provider.dart @@ -129,10 +129,17 @@ bool isSettledOrderMessage(MostroMessage? message, {DateTime? now}) { return (now ?? DateTime.now()).difference(at) > settledOrderGrace; } +/// Largest value `DateTime.fromMillisecondsSinceEpoch` accepts; anything +/// past it throws a `RangeError`. +const int _maxMillisecondsSinceEpoch = 8640000000000000; + /// The daemon sends seconds, the app fills in milliseconds when the field is -/// absent, and both units coexist in the store. +/// absent, and both units coexist in the store. A value outside the +/// `DateTime` range is treated as unknown rather than allowed to abort +/// initialization. DateTime? _messageTime(int? raw) { if (raw == null || raw <= 0) return null; final ms = raw < 1000000000000 ? raw * 1000 : raw; + if (ms > _maxMillisecondsSinceEpoch) return null; return DateTime.fromMillisecondsSinceEpoch(ms); } diff --git a/test/shared/providers/app_init_provider_test.dart b/test/shared/providers/app_init_provider_test.dart index dae3ecb8e..9608d82c4 100644 --- a/test/shared/providers/app_init_provider_test.dart +++ b/test/shared/providers/app_init_provider_test.dart @@ -98,5 +98,14 @@ void main() { expect(isSettledOrderMessage(message, now: now), isFalse, reason: 'one hour old: still inside the grace window'); }); + + test('a timestamp beyond the DateTime range counts as live, not a crash', + () { + // DateTime.fromMillisecondsSinceEpoch throws past 8640000000000000 ms; + // a corrupt stored value must not abort app initialization. + final message = withStatus(Status.expired); + message.timestamp = 8640000000000001; + expect(isSettledOrderMessage(message, now: now), isFalse); + }); }); }