Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
101 changes: 52 additions & 49 deletions lib/features/mailbox/data/network/mailbox_isolate_worker.dart
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,8 @@ class MailboxIsolateWorker {

final ThreadAPI _threadApi;
final EmailAPI _emailApi;
final Executor _isolateExecutor;

MailboxIsolateWorker(this._threadApi, this._emailApi, this._isolateExecutor);
MailboxIsolateWorker(this._threadApi, this._emailApi);

Future<List<EmailId>> markAsMailboxRead(
Session session,
Expand All @@ -63,32 +62,30 @@ class MailboxIsolateWorker {
throw const CanNotGetRootIsolateToken();
}

final result = await _isolateExecutor.execute(
arg1: MailboxMarkAsReadArguments(
session,
_threadApi,
_emailApi,
accountId,
mailboxId,
rootIsolateToken
),
fun1: _handleMarkAsMailboxReadAction,
notification: (value) {
if (value is List<EmailId>) {
log('MailboxIsolateWorker::markAsMailboxRead(): onUpdateProgress: PERCENT ${value.length / totalEmailUnread}');
onProgressController.add(Right(UpdatingMarkAsMailboxReadState(
mailboxId: mailboxId,
totalUnread: totalEmailUnread,
countRead: value.length)));
}
});
return result;
final args = MailboxMarkAsReadArguments(
session,
_threadApi,
_emailApi,
accountId,
mailboxId,
rootIsolateToken,
);
return await workerManager.executeWithPort<List<EmailId>, int>(
_buildMarkAsReadClosure(args),
onMessage: (countRead) {
log('MailboxIsolateWorker::markAsMailboxRead(): onUpdateProgress: PERCENT ${countRead / totalEmailUnread}');
onProgressController.add(Right(UpdatingMarkAsMailboxReadState(
mailboxId: mailboxId,
totalUnread: totalEmailUnread,
countRead: countRead)));
},
);
}
}

static Future<List<EmailId>> _handleMarkAsMailboxReadAction(
MailboxMarkAsReadArguments args,
TypeSendPort sendPort
MailboxMarkAsReadArguments args,
SendPort sendPort,
) async {
final rootIsolateToken = args.isolateToken;
BackgroundIsolateBinaryMessenger.ensureInitialized(rootIsolateToken);
Expand Down Expand Up @@ -144,7 +141,7 @@ class MailboxIsolateWorker {

log('MailboxIsolateWorker::_handleMarkAsMailboxRead(): MARK_READ: ${result.emailIdsSuccess.length}');
emailIdsCompleted.addAll(result.emailIdsSuccess);
sendPort.send(emailIdsCompleted);
sendPort.send(emailIdsCompleted.length);
}
}
log('MailboxIsolateWorker::_handleMarkAsMailboxRead(): TOTAL_READ: ${emailIdsCompleted.length}');
Expand Down Expand Up @@ -230,29 +227,27 @@ class MailboxIsolateWorker {
throw const CanNotGetRootIsolateToken();
}

final countEmailsCompleted = await _isolateExecutor.execute(
arg1: MoveFolderContentIsolateArguments(
session: session,
accountId: accountId,
threadAPI: _threadApi,
emailAPI: _emailApi,
currentMailboxId: request.mailboxId,
destinationMailboxId: request.destinationMailboxId,
isolateToken: rootIsolateToken,
markAsRead: request.markAsRead,
),
fun1: _moveFolderContentIsolateMethod,
notification: (value) {
if (value is int) {
log('$runtimeType::moveFolderContent(): Progress percent is ${value / request.totalEmails}');
onProgressController?.add(
Right<Failure, Success>(MoveFolderContentProgressState(
request.mailboxId,
value,
request.totalEmails,
)),
);
}
final args = MoveFolderContentIsolateArguments(
session: session,
accountId: accountId,
threadAPI: _threadApi,
emailAPI: _emailApi,
currentMailboxId: request.mailboxId,
destinationMailboxId: request.destinationMailboxId,
isolateToken: rootIsolateToken,
markAsRead: request.markAsRead,
);
final countEmailsCompleted = await workerManager.executeWithPort<int, int>(
_buildMoveFolderClosure(args),
onMessage: (value) {
log('$runtimeType::moveFolderContent(): Progress percent is ${value / request.totalEmails}');
onProgressController?.add(
Right<Failure, Success>(MoveFolderContentProgressState(
request.mailboxId,
value,
request.totalEmails,
)),
);
},
);

Expand All @@ -263,9 +258,17 @@ class MailboxIsolateWorker {
}
}

static Future<List<EmailId>> Function(SendPort) _buildMarkAsReadClosure(
MailboxMarkAsReadArguments args,
) => (sendPort) => _handleMarkAsMailboxReadAction(args, sendPort);

static Future<int> Function(SendPort) _buildMoveFolderClosure(
MoveFolderContentIsolateArguments args,
) => (sendPort) => _moveFolderContentIsolateMethod(args, sendPort);

static Future<int> _moveFolderContentIsolateMethod(
MoveFolderContentIsolateArguments args,
TypeSendPort sendPort,
SendPort sendPort,
) async {
final rootIsolateToken = args.isolateToken;
BackgroundIsolateBinaryMessenger.ensureInitialized(rootIsolateToken);
Expand Down
43 changes: 22 additions & 21 deletions lib/features/thread/data/network/thread_isolate_worker.dart
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,8 @@ import 'package:worker_manager/worker_manager.dart';
class ThreadIsolateWorker {
final ThreadAPI _threadAPI;
final EmailAPI _emailAPI;
final Executor _isolateExecutor;

ThreadIsolateWorker(this._threadAPI, this._emailAPI, this._isolateExecutor);
ThreadIsolateWorker(this._threadAPI, this._emailAPI);

Future<List<EmailId>> emptyMailboxFolder(
Session session,
Expand All @@ -49,23 +48,21 @@ class ThreadIsolateWorker {
throw const CanNotGetRootIsolateToken();
}

final result = await _isolateExecutor.execute(
arg1: EmptyMailboxFolderArguments(
session,
_threadAPI,
_emailAPI,
accountId,
mailboxId,
rootIsolateToken
),
fun1: _emptyMailboxFolderAction,
notification: (value) {
if (value is List<EmailId>) {
log('ThreadIsolateWorker::emptyMailboxFolder(): processed ${value.length} - totalEmails $totalEmails');
onProgressController.add(Right<Failure, Success>(EmptyingFolderState(
mailboxId, value.length, totalEmails
)));
}
final args = EmptyMailboxFolderArguments(
session,
_threadAPI,
_emailAPI,
accountId,
mailboxId,
rootIsolateToken,
);
final result = await workerManager.executeWithPort<List<EmailId>, int>(
_buildEmptyMailboxClosure(args),
onMessage: (processedCount) {
log('ThreadIsolateWorker::emptyMailboxFolder(): processed $processedCount - totalEmails $totalEmails');
onProgressController.add(Right<Failure, Success>(EmptyingFolderState(
mailboxId, processedCount, totalEmails
)));
},
);

Expand All @@ -77,9 +74,13 @@ class ThreadIsolateWorker {
}
}

static Future<List<EmailId>> Function(SendPort) _buildEmptyMailboxClosure(
EmptyMailboxFolderArguments args,
) => (sendPort) => _emptyMailboxFolderAction(args, sendPort);

static Future<List<EmailId>> _emptyMailboxFolderAction(
EmptyMailboxFolderArguments args,
TypeSendPort sendPort
SendPort sendPort,
) async {
final rootIsolateToken = args.isolateToken;
BackgroundIsolateBinaryMessenger.ensureInitialized(rootIsolateToken);
Expand Down Expand Up @@ -119,7 +120,7 @@ class ThreadIsolateWorker {
args.accountId,
newEmailList.listEmailIds);
emailListCompleted.addAll(listEmailIdDeleted.emailIdsSuccess);
sendPort.send(emailListCompleted);
sendPort.send(emailListCompleted.length);
} else {
hasEmails = false;
}
Expand Down
40 changes: 20 additions & 20 deletions lib/features/upload/data/network/file_uploader.dart
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,10 @@ class FileUploader {
static const String filePathExtraKey = 'path';

final DioClient _dioClient;
final worker.Executor _isolateExecutor;
final FileUtils _fileUtils;

FileUploader(
this._dioClient,
this._isolateExecutor,
this._fileUtils,
);

Expand All @@ -63,31 +61,33 @@ class FileUploader {
throw const CanNotGetRootIsolateToken();
}

return await _isolateExecutor.execute(
arg1: UploadFileArguments(
_dioClient,
_fileUtils,
uploadId,
fileInfo,
uploadUri,
rootIsolateToken,
),
fun1: _handleUploadAttachmentAction,
notification: (value) {
if (value is Success) {
log('FileUploader::uploadAttachment(): onUpdateProgress: $value');
onSendController?.add(Right(value));
}
}
final args = UploadFileArguments(
_dioClient,
_fileUtils,
uploadId,
fileInfo,
uploadUri,
rootIsolateToken,
);
return await worker.workerManager.executeWithPort<Attachment, Success>(
_buildUploadClosure(args),
onMessage: (value) {
log('FileUploader::uploadAttachment(): onUpdateProgress: $value');
onSendController?.add(Right(value));
Comment on lines +64 to +76

@coderabbitai coderabbitai Bot Apr 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n[1] FileUploader cancelToken usage\n'
rg -n -C2 --type=dart '\bcancelToken\b|executeWithPort|dioClient\.post\(' lib/features/upload/data/network/file_uploader.dart

printf '\n[2] Upload flows that pass or observe cancellation\n'
rg -n -C2 --type=dart 'uploadAttachment\(|CancelToken|isCancelled|\.cancel\(' lib/features/upload

Repository: linagora/tmail-flutter

Length of output: 8102


Non-web uploads ignore cancelToken in the isolate-backed request flow.

The method accepts cancelToken (line 46) and the web branch passes it to _dioClient.post() (line 189), but the non-web worker path does not. At line 110, _handleUploadAttachmentAction() calls dioClient.post() without a cancellation token. This allows callers to cancel uploads and update local state while the HTTP request continues in the worker isolate and completes on the server.

Options: implement cancellation via a dedicated worker-cancel channel or fall back to the main isolate for cancellable uploads.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/features/upload/data/network/file_uploader.dart` around lines 64 - 76,
The non-web worker path doesn't propagate the cancelToken so uploads can't be
aborted; propagate the cancelToken into the isolate flow by adding it to
UploadFileArguments (used by _buildUploadClosure) and then use that token inside
the worker handler (_handleUploadAttachmentAction) when calling _dioClient.post
so the request is cancellable; if your worker/executeWithPort API cannot cancel
running isolates, detect a non-null cancelToken and run the upload on the main
isolate instead (i.e., avoid the worker.path) so the existing cancelToken-based
cancellation logic is honored.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implement a dedicated worker-cancel channel is costly, and fall back to main isolate just for the cancel capability affect the main isolate performance.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tddang-linagora, that's a fair trade-off given the substantial memory gains demonstrated in the profiling images (Android Dart/Flutter heap drops from ~375 MB to ~109 MB, iOS from ~297 MB to ~105 MB). Introducing cross-isolate cancellation plumbing or forcing the main isolate to handle uploads just to support cancellation would offset those wins.

A minimal mitigation would be adding a short comment near the executeWithPort call so future maintainers understand the deliberate limitation:

// Note: cancelToken is not propagated into the worker isolate.
// Cross-isolate cancellation is not supported here due to cost/complexity;
// cancelling the upload task via the caller will not abort the in-flight HTTP request.
return await worker.workerManager.executeWithPort<Attachment, Success>(
  _buildUploadClosure(args),
  onMessage: (value) {
    log('FileUploader::uploadAttachment(): onUpdateProgress: $value');
    onSendController?.add(Right(value));
  },
)

This keeps the code self-documenting without adding any runtime overhead. Up to you whether you want to add it.


✏️ Learnings added
Learnt from: tddang-linagora
Repo: linagora/tmail-flutter PR: 4435
File: lib/features/upload/data/network/file_uploader.dart:64-76
Timestamp: 2026-04-07T03:15:49.896Z
Learning: In linagora/tmail-flutter, the non-web upload path in `lib/features/upload/data/network/file_uploader.dart` deliberately does NOT propagate `cancelToken` into the `worker.workerManager.executeWithPort` isolate call. This is an accepted trade-off: implementing a worker-cancel channel was considered too costly, and falling back to the main isolate for cancellable uploads was rejected due to main-isolate performance impact. Do not flag the missing cancelToken propagation as a bug.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


🧠 Learnings used
Learnt from: dab246
Repo: linagora/tmail-flutter PR: 4362
File: lib/features/login/domain/exceptions/authentication_exception.dart:54-62
Timestamp: 2026-03-09T03:46:24.006Z
Learning: In tmail-flutter, `DownloadAttachmentHasTokenExpiredException` (lib/features/login/domain/exceptions/authentication_exception.dart) intentionally does NOT override `props` to include `refreshToken`, for security reasons — the token must not be leaked via equality checks, debug output, or crash reporters like Sentry.

Learnt from: dab246
Repo: linagora/tmail-flutter PR: 4422
File: lib/features/mailbox_dashboard/presentation/mixin/setup_preferences_setting_mixin.dart:24-30
Timestamp: 2026-03-27T11:00:56.451Z
Learning: In linagora/tmail-flutter, the `collapseThreads` getter in `lib/features/mailbox_dashboard/presentation/mixin/setup_preferences_setting_mixin.dart` does NOT require a server Session capability check. It is enabled purely based on two conditions: (1) the in-app thread setting is enabled (`isThreadDetailEnabled == true`) and (2) `forceEmailQuery` is active (`PlatformInfo.isWeb && AppConfig.isForceEmailQueryEnabled`). Do not flag the absence of a session capability gate as a bug for this getter.

Learnt from: dab246
Repo: linagora/tmail-flutter PR: 4380
File: docs/adr/0072-thread-aware-bulk-actions-email-list-collapse-threads.md:265-314
Timestamp: 2026-03-17T10:08:56.785Z
Learning: In linagora/tmail-flutter, the thread-aware interactors introduced in ADR-0072 (e.g., MoveThreadInteractor, MarkAsThreadReadInteractor in docs/adr/0072-thread-aware-bulk-actions-email-list-collapse-threads.md) are intentionally designed to call EmailRepository directly, rather than delegating to existing email interactors (e.g., MoveMultipleEmailToMailboxInteractor, MarkAsMultipleEmailReadInteractor). This is a deliberate choice to avoid affecting old logic. Do not suggest refactoring them to reuse existing interactors.

Learnt from: dab246
Repo: linagora/tmail-flutter PR: 4194
File: lib/features/manage_account/presentation/manage_account_dashboard_controller.dart:174-176
Timestamp: 2025-12-09T09:36:45.349Z
Learning: In Dart/Flutter projects using GetX, do not wrap getBinding<T>() calls in try/catch since they return null when not found. Only wrap Get.find<T>() calls in try/catch because they throw if a dependency is unavailable. When a binding is optional, consider checking Get.isRegistered<T>() or handling the null/exception path gracefully instead of blindly catching, and document the expectation for failure modes where a dependency may not be registered.

Learnt from: dab246
Repo: linagora/tmail-flutter PR: 4315
File: core/lib/utils/app_logger.dart:140-144
Timestamp: 2026-02-09T07:57:07.651Z
Learning: For the tmail-flutter repository, ensure that logTrace calls are used selectively for diagnostic purposes related to Sentry submissions and not for regular production logging. During reviews, verify that trace-level logs are placed only where they meaningfully aid diagnosis, avoid noisy production reports, and ensure no sensitive data is logged.

},
)
.then((value) => value)
.catchError((error) => throw error);
}
}

static Future<Attachment> Function(worker.SendPort) _buildUploadClosure(
UploadFileArguments args,
) => (sendPort) => _handleUploadAttachmentAction(args, sendPort);

static Future<Attachment> _handleUploadAttachmentAction(
UploadFileArguments argsUpload,
worker.TypeSendPort sendPort
UploadFileArguments argsUpload,
worker.SendPort sendPort,
) async {
try {
final rootIsolateToken = argsUpload.isolateToken;
Expand Down
2 changes: 0 additions & 2 deletions lib/main/bindings/network/network_bindings.dart
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ import 'package:tmail_ui_user/main/exceptions/thrower/remote_exception_thrower.d
import 'package:tmail_ui_user/main/exceptions/thrower/send_email_exception_thrower.dart';
import 'package:tmail_ui_user/main/utils/ios_sharing_manager.dart';
import 'package:uuid/uuid.dart';
import 'package:worker_manager/worker_manager.dart';

class NetworkBindings extends Bindings {

Expand Down Expand Up @@ -85,7 +84,6 @@ class NetworkBindings extends Bindings {
Get.find<OIDCHttpClient>(),
Get.find<MailboxCacheManager>(),
));
Get.put(Executor());
}

void _bindingInterceptors() {
Expand Down
4 changes: 0 additions & 4 deletions lib/main/bindings/network/network_isolate_binding.dart
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import 'package:tmail_ui_user/features/upload/data/network/file_uploader.dart';
import 'package:tmail_ui_user/main/bindings/network/binding_tag.dart';
import 'package:tmail_ui_user/main/utils/ios_sharing_manager.dart';
import 'package:uuid/uuid.dart';
import 'package:worker_manager/worker_manager.dart';

class NetworkIsolateBindings extends Bindings {

Expand Down Expand Up @@ -96,16 +95,13 @@ class NetworkIsolateBindings extends Bindings {
Get.put(ThreadIsolateWorker(
Get.find<ThreadAPI>(tag: PlatformInfo.isMobile ? BindingTag.isolateTag : null),
Get.find<EmailAPI>(tag: PlatformInfo.isMobile ? BindingTag.isolateTag : null),
Get.find<Executor>(),
));
Get.put(MailboxIsolateWorker(
Get.find<ThreadAPI>(tag: PlatformInfo.isMobile ? BindingTag.isolateTag : null),
Get.find<EmailAPI>(tag: PlatformInfo.isMobile ? BindingTag.isolateTag : null),
Get.find<Executor>(),
));
Get.put(FileUploader(
Get.find<DioClient>(tag: PlatformInfo.isMobile ? BindingTag.isolateTag : null),
Get.find<Executor>(),
Get.find<FileUtils>(),
));
}
Expand Down
4 changes: 1 addition & 3 deletions lib/main/main_entry.dart
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
import 'package:core/presentation/utils/theme_utils.dart';
import 'package:core/utils/build_utils.dart';
import 'package:core/utils/config/env_loader.dart';
import 'package:core/utils/platform_info.dart';
import 'package:flutter/widgets.dart';
import 'package:get/get.dart';
import 'package:tmail_ui_user/features/caching/config/hive_cache_config.dart';
import 'package:tmail_ui_user/main.dart';
import 'package:tmail_ui_user/main/bindings/main_bindings.dart';
Expand All @@ -27,7 +25,7 @@ Future<void> runTmailPreload() async {
if (PlatformInfo.isWeb) AssetPreloader.preloadHtmlEditorAssets(),
], eagerError: false);

await Get.find<Executor>().warmUp(log: BuildUtils.isDebugMode);
await workerManager.init(dynamicSpawning: true);
Comment thread
dab246 marked this conversation as resolved.
Outdated
Comment thread
dab246 marked this conversation as resolved.
Outdated
await CozyIntegration.integrateCozy();
await HiveCacheConfig.instance.initializeEncryptionKey();

Expand Down
11 changes: 6 additions & 5 deletions pubspec.lock
Original file line number Diff line number Diff line change
Expand Up @@ -2583,11 +2583,12 @@ packages:
worker_manager:
dependency: "direct main"
description:
name: worker_manager
sha256: "42501e49ee0acad9eeda562984e3dcfe6fe3d26f2d8dc410bd76308a86447eb5"
url: "https://pub.dev"
source: hosted
version: "5.0.3"
path: "."
ref: "hotfix/worker-init-memory-leak"
resolved-ref: dd04544217c9fcc08b2a32634583f38d22cc2309
url: "https://github.com/linagora/worker_manager.git"
source: git
version: "7.2.7"
workmanager:
dependency: "direct main"
description:
Expand Down
6 changes: 5 additions & 1 deletion pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,11 @@ dependencies:

percent_indicator: 4.2.2

worker_manager: 5.0.3
# TODO: Replace with upstream when https://github.com/dsrenesanse/worker_manager/pull/123 is merged
worker_manager:
git:
url: https://github.com/linagora/worker_manager.git
ref: hotfix/worker-init-memory-leak

async: 2.13.0

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ import 'package:tmail_ui_user/main/universal_import/html_stub.dart';
import 'package:tmail_ui_user/main/utils/toast_manager.dart';
import 'package:tmail_ui_user/main/utils/twake_app_manager.dart';
import 'package:uuid/uuid.dart';
import 'package:worker_manager/worker_manager.dart';

import 'identity_creator_controller_test.mocks.dart';

Expand All @@ -76,7 +75,6 @@ import 'identity_creator_controller_test.mocks.dart';
MockSpec<GetAllIdentitiesInteractor>(),
MockSpec<IdentityUtils>(),
MockSpec<DioClient>(),
MockSpec<Executor>(),
MockSpec<FileUtils>(),
MockSpec<FileUploader>(),
MockSpec<RemoteExceptionThrower>(),
Expand Down Expand Up @@ -159,7 +157,6 @@ void main() {
mockSaveIdentityCacheOnWebInteractor = MockSaveIdentityCacheOnWebInteractor();

Get.put<DioClient>(MockDioClient(), tag: BindingTag.isolateTag);
Get.put<Executor>(MockExecutor());
Get.put<FileUtils>(MockFileUtils());
Get.put<FileUploader>(MockFileUploader());
Get.put<RemoteExceptionThrower>(MockRemoteExceptionThrower());
Expand Down
Loading