Skip to content

fix: crash when KomodoDefiSdk is disposed during periodic fetch #3117

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
CharlVS merged 4 commits into from
Sep 11, 2025
Merged

fix: crash when KomodoDefiSdk is disposed during periodic fetch #3117

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
92295b0
fix crash when KomodoDefiSdk is disposed during periodic fetch
DeckerSU Aug 29, 2025
5ba4fb6
Merge branch 'dev' into patch-fix-crach-on-exit
CharlVS Sep 1, 2025
58f3252
Merge branch 'dev' into patch-fix-crach-on-exit
CharlVS Sep 2, 2025
8bc1e85
Merge branch 'dev' into patch-fix-crach-on-exit
CharlVS Sep 11, 2025
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
19 changes: 17 additions & 2 deletions lib/blocs/trading_entities_bloc.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ class TradingEntitiesBloc implements BlocBase {
List<MyOrder> _myOrders = [];
List<Swap> _swaps = [];
Timer? timer;
bool _closed = false;
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Sep 1, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Stop the timer and mark closed during disposal to prevent leaks and stray ticks

Right now the periodic timer keeps firing after close and holds references. Cancel it when _closed flips to true and in dispose(). Also mark _closed in dispose() and close the controllers.

   Timer? timer;
   bool _closed = false;
@@
   @override
   void dispose() {
-    _authModeListener?.cancel();
+    _closed = true;
+    timer?.cancel();
+    timer = null;
+    _authModeListener?.cancel();
+    // Best-effort; returns Future<void>, but dispose() is sync.
+    _myOrdersController.close();
+    _swapsController.close();
   }
@@
     timer = Timer.periodic(const Duration(seconds: 1), (_) async {
-      if (_closed) return;
+      if (_closed) {
+        timer?.cancel();
+        timer = null;
+        return;
+      }
@@
-      } catch (e) {
-        if (e is StateError && e.message.contains('disposed')) {
-          _closed = true;
-        } else {
+      } catch (e) {
+        if (e is StateError && e.message.contains('disposed')) {
+          _closed = true;
+          timer?.cancel();
+          timer = null;
+          return;
+        } else {
           await log(
             'fetch error: $e',
             path: 'TradingEntitiesBloc.fetch',
           );
         }
       } finally {

Also applies to: 72-75, 81-81, 89-91

🤖 Prompt for AI Agents 
In lib/blocs/trading_entities_bloc.dart around line 39 (and similarly lines
72-75, 81, 89-91), the periodic Timer is never cancelled and _closed is never
set during disposal; update the class so that when you set _closed = true you
also cancel the Timer (if not null and active), and in dispose() set _closed =
true, cancel the Timer, and close any StreamControllers/subjects used by the
bloc; ensure any periodic callbacks check _closed before running and that all
controller.close() calls are invoked in dispose to prevent leaks and stray
ticks.


Comment on lines +39 to 40
Copy link

Copilot AI Sep 1, 2025

Choose a reason for hiding this comment

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

The _closed flag lacks a proper disposal mechanism. Consider adding a dispose() method that sets _closed = true and cancels the timer to ensure clean resource cleanup.

Suggested change
bool _closed = false;
/// Dispose resources and mark bloc as closed.
void dispose() {
_closed = true;
timer?.cancel();
_authModeListener?.cancel();
_myOrdersController.close();
_swapsController.close();
}

Copilot uses AI. Check for mistakes.
final StreamController<List<MyOrder>> _myOrdersController =
StreamController<List<MyOrder>>.broadcast();
Expand All @@ -61,6 +62,7 @@ class TradingEntitiesBloc implements BlocBase {
}

Future<void> fetch() async {
if (_closed) return;
if (!await _kdfSdk.auth.isSignedIn()) return;

myOrders = await _myOrdersService.getOrders() ?? [];
Expand All @@ -76,12 +78,25 @@ class TradingEntitiesBloc implements BlocBase {
bool updateInProgress = false;

timer = Timer.periodic(const Duration(seconds: 1), (_) async {
if (_closed) return;
Copy link
Copy Markdown

@cursor cursor Bot Sep 1, 2025

Choose a reason for hiding this comment

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

Bug: Bloc Dispose Fails to Cancel Timer

The dispose() method doesn't set _closed = true or cancel the periodic timer. This allows the timer to continue running after the bloc is disposed, bypassing the _closed checks and potentially causing StateError crashes by attempting to access already-disposed resources.

Fix in Cursor Fix in Web

if (updateInProgress) return;
// TODO!: do not run for hidden login or HW

updateInProgress = true;
await fetch();
updateInProgress = false;
try {
await fetch();
} catch (e) {
if (e is StateError && e.message.contains('disposed')) {
Comment on lines 85 to +89
Copy link

Copilot AI Sep 1, 2025

Choose a reason for hiding this comment

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

The error message check is fragile and could miss disposal errors with different wording. Consider checking the error type more specifically or using a more robust method to detect SDK disposal state.

Suggested change
updateInProgress = true;
await fetch();
updateInProgress = false;
try {
await fetch();
} catch (e) {
if (e is StateError && e.message.contains('disposed')) {
// Check SDK disposal state before running fetch
if (_kdfSdk is Disposable && (_kdfSdk as dynamic).isDisposed == true) {
_closed = true;
return;
}
updateInProgress = true;
try {
await fetch();
} catch (e) {
if (e is StateError) {

Copilot uses AI. Check for mistakes.
_closed = true;
} else {
await log(
'fetch error: $e',
path: 'TradingEntitiesBloc.fetch',
);
}
} finally {
updateInProgress = false;
}
});
}

Expand Down
Loading