diff --git a/examples/database_crud/lib/main.dart b/examples/database_crud/lib/main.dart index 0d2756867..addac3867 100644 --- a/examples/database_crud/lib/main.dart +++ b/examples/database_crud/lib/main.dart @@ -158,27 +158,9 @@ class _TasksPageState extends State { } Future _rename(Task task) async { - final controller = TextEditingController(text: task.title); final title = await showDialog( context: context, - builder: (context) => AlertDialog( - title: const Text('Rename task'), - content: TextField( - controller: controller, - autofocus: true, - decoration: const InputDecoration(labelText: 'Title'), - ), - actions: [ - TextButton( - onPressed: () => Navigator.pop(context), - child: const Text('Cancel'), - ), - FilledButton( - onPressed: () => Navigator.pop(context, controller.text.trim()), - child: const Text('Save'), - ), - ], - ), + builder: (context) => _RenameDialog(initialTitle: task.title), ); if (title == null || title.isEmpty) return; await _mutate(() => _repository.renameTask(id: task.id, title: title)); @@ -374,6 +356,52 @@ class _TaskFormResult { final Priority priority; } +/// Asks for a new title for an existing task. +/// +/// Owns its [TextEditingController] so that it is disposed together with the +/// dialog. Creating the controller in the caller instead would either leak it +/// or dispose it while the route is still animating out. +class _RenameDialog extends StatefulWidget { + const _RenameDialog({required this.initialTitle}); + + final String initialTitle; + + @override + State<_RenameDialog> createState() => _RenameDialogState(); +} + +class _RenameDialogState extends State<_RenameDialog> { + late final _title = TextEditingController(text: widget.initialTitle); + + @override + void dispose() { + _title.dispose(); + super.dispose(); + } + + @override + Widget build(BuildContext context) { + return AlertDialog( + title: const Text('Rename task'), + content: TextField( + controller: _title, + autofocus: true, + decoration: const InputDecoration(labelText: 'Title'), + ), + actions: [ + TextButton( + onPressed: () => Navigator.pop(context), + child: const Text('Cancel'), + ), + FilledButton( + onPressed: () => Navigator.pop(context, _title.text.trim()), + child: const Text('Save'), + ), + ], + ); + } +} + class _TaskDialog extends StatefulWidget { const _TaskDialog({required this.projects}); diff --git a/examples/passkeys/lib/main.dart b/examples/passkeys/lib/main.dart index 0f54ae405..a5e166d5c 100644 --- a/examples/passkeys/lib/main.dart +++ b/examples/passkeys/lib/main.dart @@ -159,6 +159,52 @@ class _SignInViewState extends State { } } +/// Asks for a new friendly name for an existing passkey. +/// +/// Owns its [TextEditingController] so that it is disposed together with the +/// dialog. Creating the controller in the caller instead would either leak it +/// or dispose it while the route is still animating out. +class _RenameDialog extends StatefulWidget { + const _RenameDialog({required this.initialName}); + + final String initialName; + + @override + State<_RenameDialog> createState() => _RenameDialogState(); +} + +class _RenameDialogState extends State<_RenameDialog> { + late final _name = TextEditingController(text: widget.initialName); + + @override + void dispose() { + _name.dispose(); + super.dispose(); + } + + @override + Widget build(BuildContext context) { + return AlertDialog( + title: const Text('Rename passkey'), + content: TextField( + controller: _name, + autofocus: true, + decoration: const InputDecoration(labelText: 'Friendly name'), + ), + actions: [ + TextButton( + onPressed: () => Navigator.pop(context), + child: const Text('Cancel'), + ), + FilledButton( + onPressed: () => Navigator.pop(context, _name.text.trim()), + child: const Text('Save'), + ), + ], + ); + } +} + /// Lists, registers, renames and deletes passkeys for the signed in user. class SignedInView extends StatefulWidget { const SignedInView({super.key}); @@ -201,27 +247,10 @@ class _SignedInViewState extends State { } Future _rename(Passkey passkey) async { - final controller = TextEditingController(text: passkey.friendlyName); final name = await showDialog( context: context, - builder: (context) => AlertDialog( - title: const Text('Rename passkey'), - content: TextField( - controller: controller, - autofocus: true, - decoration: const InputDecoration(labelText: 'Friendly name'), - ), - actions: [ - TextButton( - onPressed: () => Navigator.pop(context), - child: const Text('Cancel'), - ), - FilledButton( - onPressed: () => Navigator.pop(context, controller.text), - child: const Text('Save'), - ), - ], - ), + builder: (context) => + _RenameDialog(initialName: passkey.friendlyName ?? ''), ); if (name == null || name.isEmpty) return; try { diff --git a/packages/supabase_flutter/lib/src/supabase.dart b/packages/supabase_flutter/lib/src/supabase.dart index 34500c514..f73bb49b2 100644 --- a/packages/supabase_flutter/lib/src/supabase.dart +++ b/packages/supabase_flutter/lib/src/supabase.dart @@ -171,10 +171,22 @@ class Supabase { /// Whether the Supabase instance has been initialized. Useful for debugging. bool get isInitialized => _isInitialized; + SupabaseClient? _client; + /// The supabase client for this instance /// - /// Throws an error if [Supabase.initialize] was not called. - late SupabaseClient client; + /// Throws a [StateError] if [Supabase.initialize] was not called, or if the + /// instance has since been disposed. + SupabaseClient get client { + final currentClient = _client; + if (currentClient == null) { + throw StateError( + 'You must initialize the supabase instance before calling ' + 'Supabase.instance.client', + ); + } + return currentClient; + } SupabaseAuth? _supabaseAuth; @@ -202,14 +214,56 @@ class Supabase { StreamSubscription? _logSubscription; /// Dispose the instance to free up resources. + /// + /// Calling this on an instance that is not initialized does nothing, so it + /// is safe to call more than once. Future dispose() async { - _targetLifecycleState = null; - await _restoreSessionCancellableOperation?.cancel(); - await _logSubscription?.cancel(); - await client.dispose(); - _instance._supabaseAuth?.dispose(); - _lifecycleListener?.dispose(); + final currentClient = _client; + if (currentClient == null) return; + + final supabaseAuth = _supabaseAuth; + final lifecycleListener = _lifecycleListener; + final restoreSession = _restoreSessionCancellableOperation; + final logSubscription = _logSubscription; + final pendingLifecycleOperation = _pendingLifecycleOperation; + + _client = null; + _supabaseAuth = null; + _restoreSessionCancellableOperation = null; + _lifecycleListener = null; + _logSubscription = null; _isInitialized = false; + + _targetLifecycleState = null; + lifecycleListener?.dispose(); + + await _disposeAll([ + () => restoreSession?.cancel(), + () => logSubscription?.cancel(), + () => pendingLifecycleOperation, + currentClient.dispose, + () => supabaseAuth?.dispose(), + ]); + } + + /// Runs every step, then rethrows the first error any of them threw. + static Future _disposeAll(List Function()> steps) async { + Object? firstError; + StackTrace? firstStackTrace; + + for (final step in steps) { + try { + await step(); + } catch (error, stackTrace) { + _log.warning('Error while disposing Supabase', error, stackTrace); + firstError ??= error; + firstStackTrace ??= stackTrace; + } + } + + if (firstError != null) { + Error.throwWithStackTrace(firstError, firstStackTrace!); + } } void _init( @@ -228,7 +282,7 @@ class Supabase { ...Constants.defaultHeaders, ...?customHeaders, }; - client = SupabaseClient( + final newClient = _client = SupabaseClient( supabaseUrl, supabaseKey, httpClient: httpClient, @@ -245,7 +299,7 @@ class Supabase { // flutter web hot-restart. if (kDebugMode) { disposePreviousClient(); - markClientToDispose(client); + markClientToDispose(newClient); } _setupLifecycleListener(); diff --git a/packages/supabase_flutter/test/initialization_test.dart b/packages/supabase_flutter/test/initialization_test.dart index 50b285663..a2608feaa 100644 --- a/packages/supabase_flutter/test/initialization_test.dart +++ b/packages/supabase_flutter/test/initialization_test.dart @@ -143,6 +143,32 @@ void main() { ); }); + test('dispose drops the reference to the client', () async { + await Supabase.initialize( + url: supabaseUrl, + publishableKey: supabaseKey, + debug: false, + ); + + final supabase = Supabase.instance; + await supabase.dispose(); + + expect(() => supabase.client, throwsStateError); + }); + + test('dispose can be called more than once', () async { + await Supabase.initialize( + url: supabaseUrl, + publishableKey: supabaseKey, + debug: false, + ); + + final supabase = Supabase.instance; + await supabase.dispose(); + + await expectLater(supabase.dispose(), completes); + }); + test('handles multiple initializations correctly', () async { await Supabase.initialize( url: supabaseUrl, diff --git a/packages/supabase_flutter/test/widget_test.dart b/packages/supabase_flutter/test/widget_test.dart index c677ee3df..d11111b31 100644 --- a/packages/supabase_flutter/test/widget_test.dart +++ b/packages/supabase_flutter/test/widget_test.dart @@ -15,16 +15,19 @@ void main() { testWidgets('Signing out triggers AuthChangeEvent.signedOut event', ( tester, ) async { - // Initialize the Supabase singleton - await Supabase.initialize( - url: supabaseUrl, - publishableKey: supabaseKey, - debug: false, - authOptions: FlutterAuthClientOptions( - localStorage: const MockLocalStorage(), - pkceAsyncStorage: MockAsyncStorage(), + await tester.runAsync( + () => Supabase.initialize( + url: supabaseUrl, + publishableKey: supabaseKey, + debug: false, + authOptions: FlutterAuthClientOptions( + localStorage: const MockLocalStorage(), + pkceAsyncStorage: MockAsyncStorage(), + ), ), ); + addTearDown(() => tester.runAsync(() => Supabase.instance.dispose())); + Supabase.instance.client.auth.stopAutoRefresh(); await tester.pumpWidget(const MaterialApp(home: MockWidget())); await tester.tap(find.text('Sign out')); diff --git a/packages/supabase_flutter/test/widget_test_stubs.dart b/packages/supabase_flutter/test/widget_test_stubs.dart index 298b40e79..abc1b9b0c 100644 --- a/packages/supabase_flutter/test/widget_test_stubs.dart +++ b/packages/supabase_flutter/test/widget_test_stubs.dart @@ -19,6 +19,7 @@ class MockWidget extends StatefulWidget { class _MockWidgetState extends State { bool isSignedIn = true; + StreamSubscription? _authSubscription; @override Widget build(BuildContext context) { @@ -37,7 +38,9 @@ class _MockWidgetState extends State { @override void initState() { super.initState(); - Supabase.instance.client.auth.onAuthStateChange.listen((data) { + _authSubscription = Supabase.instance.client.auth.onAuthStateChange.listen(( + data, + ) { if (data.event == AuthChangeEvent.signedOut) { setState(() { isSignedIn = false; @@ -45,6 +48,12 @@ class _MockWidgetState extends State { } }); } + + @override + void dispose() { + unawaited(_authSubscription?.cancel()); + super.dispose(); + } } /// Local storage that returns an expired session diff --git a/packages/yet_another_json_isolate/lib/src/_isolates_io.dart b/packages/yet_another_json_isolate/lib/src/_isolates_io.dart index 2bf5749ff..5502f3572 100644 --- a/packages/yet_another_json_isolate/lib/src/_isolates_io.dart +++ b/packages/yet_another_json_isolate/lib/src/_isolates_io.dart @@ -20,11 +20,23 @@ class YAJsonIsolate { final _createdIsolate = Completer(); late final _events = StreamQueue(_receivePort); bool _hasStartedInitialize = false; + Future? _disposal; + + bool get _isDisposed => _disposal != null; + + void _throwIfDisposed() { + if (_isDisposed) { + throw StateError('This YAJsonIsolate has already been disposed.'); + } + } /// Initialize the isolate /// - /// This method is called automatically when the first method is called. Manually initializing before first json de/encode can improve performance. + /// This method is called automatically when the first method is called. + /// Manually initializing before the first JSON decode or encode can improve + /// performance. Future initialize() async { + _throwIfDisposed(); assert( _hasStartedInitialize == false, 'initialize() can only be called once per isolate.', @@ -43,8 +55,18 @@ class YAJsonIsolate { /// Dispose the isolate /// - /// This exits the isolate - Future dispose() async { + /// This exits the isolate. Safe to call more than once, and safe to call on + /// an instance that was never used. Concurrent calls all await the same + /// shutdown, so awaiting any of them means the isolate is gone. Using the + /// instance afterwards throws a [StateError]. + Future dispose() => _disposal ??= _dispose(); + + Future _dispose() async { + if (!_hasStartedInitialize) { + _receivePort.close(); + return; + } + await _createdIsolate.future; _sendPort.send(null); _receivePort.close(); @@ -52,6 +74,7 @@ class YAJsonIsolate { } Future decode(String json) async { + _throwIfDisposed(); if (!_createdIsolate.isCompleted) { if (!_hasStartedInitialize) await initialize(); await _createdIsolate.future; @@ -61,6 +84,7 @@ class YAJsonIsolate { } Future encode(Object? json) async { + _throwIfDisposed(); if (!_createdIsolate.isCompleted) { if (!_hasStartedInitialize) await initialize(); await _createdIsolate.future; diff --git a/packages/yet_another_json_isolate/test/yet_another_json_isolate_io_test.dart b/packages/yet_another_json_isolate/test/yet_another_json_isolate_io_test.dart index 8c47254fb..f2c34f9da 100644 --- a/packages/yet_another_json_isolate/test/yet_another_json_isolate_io_test.dart +++ b/packages/yet_another_json_isolate/test/yet_another_json_isolate_io_test.dart @@ -4,12 +4,17 @@ library; import 'package:test/test.dart'; import 'package:yet_another_json_isolate/yet_another_json_isolate.dart'; +/// Every disposal in this group is bounded, so a regression that makes +/// `dispose()` wait forever fails the test instead of hanging the suite. +Future dispose(YAJsonIsolate isolate) => + isolate.dispose().timeout(const Duration(seconds: 5)); + void main() { group('io implementation', () { test('throws when initialize is called twice', () async { final isolate = YAJsonIsolate(); await isolate.initialize(); - addTearDown(isolate.dispose); + addTearDown(() => dispose(isolate)); expect(isolate.initialize(), throwsA(isA())); }); @@ -17,5 +22,48 @@ void main() { final isolate = YAJsonIsolate(debugName: 'my-isolate'); expect(isolate.debugName, 'my-isolate'); }); + + test('dispose completes when the isolate was never used', () async { + final isolate = YAJsonIsolate(); + await expectLater(dispose(isolate), completes); + }); + + test('dispose completes when called twice', () async { + final isolate = YAJsonIsolate(); + await isolate.decode('{}'); + await dispose(isolate); + await expectLater(dispose(isolate), completes); + }); + + test('concurrent dispose calls all await the same shutdown', () async { + final isolate = YAJsonIsolate(); + await isolate.decode('{}'); + + final first = isolate.dispose(); + final second = isolate.dispose(); + expect(identical(first, second), isTrue); + + await expectLater( + Future.wait([first, second]).timeout(const Duration(seconds: 5)), + completes, + ); + }); + + test('using the isolate after dispose throws', () async { + final isolate = YAJsonIsolate(); + await isolate.decode('{}'); + await dispose(isolate); + + expect(isolate.decode('{}'), throwsStateError); + expect(isolate.encode({}), throwsStateError); + expect(isolate.initialize(), throwsStateError); + }); + + test('a never used isolate also rejects work after dispose', () async { + final isolate = YAJsonIsolate(); + await dispose(isolate); + + expect(isolate.decode('{}'), throwsStateError); + }); }); }