fix(desktop): hide the window before exit teardown and keep shutdown out of live UI
Closing the window now runs a graceful exit (so trackers get their terminal scrobble), but the teardown kept the window on screen for seconds while screens visibly emptied as servers disconnected. Hide the window as the first step of exit teardown, bounded so a stalled platform channel cannot hold an already-accepted exit open, and cover every graceful-exit entry (window X, Cmd+Q, OS-initiated), not just the close button. Add a terminal MultiServerManager.shutdown() that closes the status streams before draining clients: the still-mounted widget tree must never observe an "all servers gone" snapshot mid-exit, which would flip the app into offline UI and dismantle screens (and with it the teardown-driven AppLifecycleListener used-after-dispose assert). A health result landing after shutdown is a silent no-op. Supersedes the flag-based approach in #1999.
This commit is contained in:
+15
-1
@@ -1268,6 +1268,20 @@ class _MainAppState extends State<MainApp> with WidgetsBindingObserver {
|
||||
if (_shutdownStarted) return;
|
||||
_shutdownStarted = true;
|
||||
|
||||
// Hide the window before anything else so the exit reads as an instant
|
||||
// close: the teardown below runs against a still-mounted tree and its
|
||||
// state churn must never be user-visible. Cmd+Q and OS-initiated exits
|
||||
// arrive here directly, so the close-button path is not the only entry.
|
||||
// Bounded — hide() is a platform-channel round trip and a stalled
|
||||
// platform thread must not hold the already-accepted exit open.
|
||||
if (PlatformDetector.isDesktopOS()) {
|
||||
try {
|
||||
await windowManager.hide().timeout(const Duration(seconds: 1));
|
||||
} catch (e, st) {
|
||||
appLogger.w('Failed to hide window before exit teardown', error: e, stackTrace: st);
|
||||
}
|
||||
}
|
||||
|
||||
_syncDebounce?.cancel();
|
||||
await _watchStateSubscription?.cancel();
|
||||
_removeConnectivitySyncListener();
|
||||
@@ -1280,7 +1294,7 @@ class _MainAppState extends State<MainApp> with WidgetsBindingObserver {
|
||||
await TrackerCoordinator.instance.stopPlayback().timeout(const Duration(seconds: 3), onTimeout: () {});
|
||||
TrackerCoordinator.instance.cancelInFlight();
|
||||
|
||||
await _serverManager.disconnectAllGracefully();
|
||||
await _serverManager.shutdown();
|
||||
await Future.wait([
|
||||
httpClient.closeGracefully(drainTimeout: const Duration(seconds: 5)),
|
||||
closeArtworkHttpClientGracefully(),
|
||||
|
||||
@@ -78,8 +78,13 @@ class MultiServerManager {
|
||||
Stream<Map<String, bool>> get statusStream => _statusController.stream;
|
||||
|
||||
/// Publish a snapshot of the per-server online map — subscribers must never
|
||||
/// receive the live [_serverStatus] instance.
|
||||
void _emitStatus() => _statusController.add(Map.from(_serverStatus));
|
||||
/// receive the live [_serverStatus] instance. A no-op once [shutdown] or
|
||||
/// [dispose] closed the controller, so a health probe or reconnect landing
|
||||
/// mid-exit cannot throw into a closed stream.
|
||||
void _emitStatus() {
|
||||
if (_statusController.isClosed) return;
|
||||
_statusController.add(Map.from(_serverStatus));
|
||||
}
|
||||
|
||||
/// Per-server connect progress during a bind. Unlike [statusStream] — whose
|
||||
/// first emission means "the binder's first connect pass finished" and which
|
||||
@@ -1447,6 +1452,20 @@ class MultiServerManager {
|
||||
return clients;
|
||||
}
|
||||
|
||||
/// Terminal app-exit teardown: closes the status streams first, then drains
|
||||
/// every client connection.
|
||||
///
|
||||
/// Unlike [disconnectAllGracefully] — whose empty snapshot profile-switch
|
||||
/// and disconnect flows must observe — exit teardown must stay invisible:
|
||||
/// the widget tree is still mounted while the exit request is serviced, and
|
||||
/// an empty snapshot reads as "all servers gone", flipping the app into
|
||||
/// offline UI and dismantling screens mid-shutdown.
|
||||
Future<void> shutdown({Duration drainTimeout = const Duration(seconds: 5)}) async {
|
||||
if (!_statusController.isClosed) unawaited(_statusController.close());
|
||||
if (!_connectProgressController.isClosed) unawaited(_connectProgressController.close());
|
||||
await disconnectAllGracefully(drainTimeout: drainTimeout);
|
||||
}
|
||||
|
||||
/// Dispose resources
|
||||
void dispose() {
|
||||
disconnectAll();
|
||||
|
||||
@@ -1785,6 +1785,44 @@ void main() {
|
||||
});
|
||||
});
|
||||
|
||||
group('shutdown', () {
|
||||
test('clears all state without emitting a snapshot and closes the stream', () async {
|
||||
final m = MultiServerManager();
|
||||
addTearDown(m.dispose);
|
||||
|
||||
m.updateServerStatus(ServerId('a'), true);
|
||||
m.updateServerStatus(ServerId('b'), false);
|
||||
|
||||
final emitted = <Map<String, bool>>[];
|
||||
var done = false;
|
||||
final sub = m.statusStream.listen(emitted.add, onDone: () => done = true);
|
||||
addTearDown(sub.cancel);
|
||||
|
||||
await m.shutdown();
|
||||
await Future<void>.delayed(Duration.zero);
|
||||
|
||||
// Exit teardown must stay invisible: an empty snapshot would flip the
|
||||
// still-mounted UI into offline mode while the app shuts down.
|
||||
expect(m.serverIds, isEmpty);
|
||||
expect(m.onlineServerIds, isEmpty);
|
||||
expect(m.offlineServerIds, isEmpty);
|
||||
expect(emitted, isEmpty);
|
||||
expect(done, isTrue);
|
||||
});
|
||||
|
||||
test('a health result landing after shutdown is a silent no-op', () async {
|
||||
final m = MultiServerManager();
|
||||
addTearDown(m.dispose);
|
||||
|
||||
m.updateServerStatus(ServerId('a'), true);
|
||||
await m.shutdown();
|
||||
|
||||
// A probe that was in flight when the exit began must not throw into
|
||||
// the closed status controller.
|
||||
expect(() => m.updateServerStatus(ServerId('a'), false), returnsNormally);
|
||||
});
|
||||
});
|
||||
|
||||
group('dispose', () {
|
||||
test('disposing without connectivity monitoring does not throw', () {
|
||||
final m = MultiServerManager();
|
||||
|
||||
Reference in New Issue
Block a user