fix(realtime): remove only the given channel from the client - #1669
Merged
Conversation
`RealtimeClient.remove()` matched channels on `joinRef`, which is the empty string until a channel subscribes. Removing one channel that had never joined therefore removed every channel that had never joined. The dropped channels are still held by the caller but are no longer known to the client, so `removeAllChannels()` does not unsubscribe them and their rejoin timers stay armed. An empty list also made the client schedule a disconnect while channels were still in use. `remove()` is only ever called by a channel passing itself, so it now matches on identity.
github-actions
Bot
added
the
realtime
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthrough
ChangesChannel removal
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Tr00d
approved these changes
Aug 7, 2026
spydon
added a commit
that referenced
this pull request
Aug 7, 2026
Fixes three leaks found by auditing the packages and the example apps with `leak_tracker`. The tracking itself was a one-off investigation and is not part of this change. Closes SDK-1441. ## Leaks fixed **The `Supabase` singleton pinned the disposed client graph.** `dispose()` tore everything down but never dropped its references, so the disposed `SupabaseClient` and everything under it (auth, realtime, PostgREST, storage, functions, the http clients) stayed reachable from a static field for the rest of the process. `client` is now a getter over a nullable backing field, and `dispose()` clears `_client`, `_supabaseAuth`, `_lifecycleListener`, `_restoreSessionCancellableOperation`, `_logSubscription` and the pending lifecycle operation. Clearing happens before teardown, so a step that throws cannot leave the singleton pinning a half-disposed client, and `dispose()` returns early when there is nothing to dispose so it is safe to call more than once. **`YAJsonIsolate.dispose()` hung forever when the isolate was never used.** It awaited `_createdIsolate.future`, which only completes inside `initialize()`, so an instance that was never exercised never finished disposing and left its `ReceivePort` open. Reachable through `PostgrestClient` and `FunctionsClient` when they are handed a custom isolate that never runs. It now returns early after closing the port when nothing was spawned. Overlapping `dispose()` calls share a single shutdown future, and `initialize()`, `decode()` and `encode()` throw a `StateError` afterwards rather than spawning a replacement isolate onto a closed receive port and waiting for a reply that never arrives. This one also blocked the fix above: making the existing widget test dispose the singleton turned it into a 10 minute timeout until the isolate teardown was fixed. **Undisposed `TextEditingController` in the `database_crud` and `passkeys` examples.** Both rename dialogs built a controller in the calling method and never disposed it. Each is now a `_RenameDialog` stateful widget that owns and disposes its controller. Disposing in the caller does not work: `whenComplete` on the `showDialog` future fires while the route is still animating out, and the `TextField` then rebuilds against a disposed controller. ## Tests - `initialization_test.dart` gains cases asserting the singleton drops its client reference on dispose and that a second `dispose()` completes. The first captures the instance before disposing, because `Supabase.instance` itself refuses to hand out a disposed instance. - `yet_another_json_isolate` gains cases for disposing an unused isolate, disposing twice, overlapping disposals, and using the isolate after disposal. Every disposal in that suite is bounded by a timeout so a regression fails rather than hanging. - The existing widget test now tears the singleton down via `addTearDown`, which also covers the isolate fix in a widget test context. It needs `tester.runAsync` around both initialize and dispose, see the caveat below. - The `MockWidget` stub now cancels its auth subscription in `dispose()`. ## Verification - `supabase_flutter`: 65 tests pass. - `supabase`: 134 tests pass. - `yet_another_json_isolate`: 27, `postgrest`: 196, `functions_client`: 48, `realtime_client` and `storage_client` all pass. - `dcm analyze packages` is clean. - The example fixes were confirmed on an Android emulator against the local stack while the leak tracking was still wired up: `database_crud` went from one reported leak to none. `passkeys` needs real platform passkey support and does not run on an emulator, so its identical fix comes from inspection. ## Breaking change `Supabase.instance.client` is a getter rather than a field, so it can no longer be assigned, and it throws a `StateError` after `dispose()` instead of returning the disposed client. Reads on an initialized instance are unchanged. ## Found but not fixed here **`SupabaseClient.dispose()` cannot complete inside `testWidgets`.** The constructor eagerly spawns the JSON isolate, and `Isolate.spawn` never resolves under the fake clock, so consumers writing widget tests have to wrap both `Supabase.initialize` and `dispose` in `tester.runAsync`. Making `dispose()` non-blocking would weaken its contract, so it is a design call rather than a drive-by fix. The other finding from this audit, `RealtimeClient.remove()` dropping unrelated channels, is fixed separately in #1669. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Renaming tasks and passkeys now opens a dedicated dialog with the current name prefilled. * Saved names are automatically trimmed of leading and trailing spaces. * **Bug Fixes** * Improved cleanup and disposal behavior for Supabase and JSON processing resources. * Repeated or early disposal is now handled safely. * Attempts to use disposed services now provide clear state errors. * Improved cleanup during authentication and widget testing scenarios. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
RealtimeClient.remove()matched channels onjoinRef, which is the empty string until a channel subscribes. Removing one channel that had never joined therefore removed every channel that had never joined.Two consequences beyond the wrong list:
removeAllChannels()will not unsubscribe them and their rejoin timers stay armed.channelsgoing empty makes the client schedule a disconnect while channels are still in use.remove()is only ever called fromRealtimeChannel's close handler passingthis, so it now matches on identity.Tests
Two cases in the
removegroup ofsocket_test.dart, both of which fail against the old implementation:The existing case did not catch this because it uses mocked channels with distinct
joinRefvalues.Verification
realtime_client(205 tests) andsupabase(134 tests) pass.Found while auditing for leaks in #1668, which has the rest of that work.
Summary by CodeRabbit
Bug Fixes
Tests