Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
193 changes: 159 additions & 34 deletions flutter/lib/desktop/pages/terminal_tab_page.dart
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ class _TerminalTabPageState extends State<TerminalTabPage> {
static const IconData selectedIcon = Icons.terminal;
static const IconData unselectedIcon = Icons.terminal_outlined;
int _nextTerminalId = 1;
// Lightweight idempotency guard for async close operations
final Set<String> _closingTabs = {};

_TerminalTabPageState(Map<String, dynamic> params) {
Get.put(DesktopTabController(tabType: DesktopTabType.terminal));
Expand Down Expand Up @@ -70,24 +72,7 @@ class _TerminalTabPageState extends State<TerminalTabPage> {
label: tabLabel,
selectedIcon: selectedIcon,
unselectedIcon: unselectedIcon,
onTabCloseButton: () async {
if (await desktopTryShowTabAuditDialogCloseCancelled(
id: tabKey,
tabController: tabController,
)) {
return;
}
// Close the terminal session first
final ffi = TerminalConnectionManager.getExistingConnection(peerId);
if (ffi != null) {
final terminalModel = ffi.terminalModels[terminalId];
if (terminalModel != null) {
await terminalModel.closeTerminal();
}
}
// Then close the tab
tabController.closeBy(tabKey);
},
onTabCloseButton: () => _closeTab(tabKey),
page: TerminalPage(
key: ValueKey(tabKey),
id: peerId,
Expand All @@ -102,6 +87,149 @@ class _TerminalTabPageState extends State<TerminalTabPage> {
);
}

/// Unified tab close handler for all close paths (button, shortcut, programmatic).
/// Shows audit dialog, cleans up session if not persistent, then removes the UI tab.
Future<void> _closeTab(String tabKey) async {
// Idempotency guard: skip if already closing this tab
if (_closingTabs.contains(tabKey)) return;
_closingTabs.add(tabKey);

try {
// Snapshot peerTabCount BEFORE any await to avoid race with concurrent
// _closeAllTabs clearing tabController (which would make the live count
// drop to 0 and incorrectly trigger session persistence).
// Note: the snapshot may become stale if other individual tabs are closed
// during the audit dialog, but this is an acceptable trade-off.
int? snapshotPeerTabCount;
final parsed = _parseTabKey(tabKey);
if (parsed != null) {
final (peerId, _) = parsed;
snapshotPeerTabCount = tabController.state.value.tabs.where((t) {
final p = _parseTabKey(t.key);
return p != null && p.$1 == peerId;
}).length;
}

if (await desktopTryShowTabAuditDialogCloseCancelled(
id: tabKey,
tabController: tabController,
)) {
return;
}

// Close terminal session if not in persistent mode.
// Wrapped separately so session cleanup failure never blocks UI tab removal.
try {
await _closeTerminalSessionIfNeeded(tabKey,
peerTabCount: snapshotPeerTabCount);
} catch (e) {
debugPrint('[TerminalTabPage] Session cleanup failed for $tabKey: $e');
}
// Always close the tab from UI, regardless of session cleanup result
tabController.closeBy(tabKey);
} catch (e) {
debugPrint('[TerminalTabPage] Error closing tab $tabKey: $e');
} finally {
_closingTabs.remove(tabKey);
}
}

/// Close all tabs with session cleanup.
/// Used for window-level close operations (onDestroy, handleWindowCloseButton).
/// UI tabs are removed immediately; session cleanup runs in parallel with a
/// bounded timeout so window close is not blocked indefinitely.
Future<void> _closeAllTabs() async {
final tabKeys = tabController.state.value.tabs.map((t) => t.key).toList();
// Remove all UI tabs immediately (same instant behavior as the old tabController.clear())
tabController.clear();
// Run session cleanup in parallel with bounded timeout (closeTerminal() has internal 3s timeout).
// Skip tabs already being closed by a concurrent _closeTab() to avoid duplicate FFI calls.
final futures = tabKeys
.where((tabKey) => !_closingTabs.contains(tabKey))
.map((tabKey) async {
try {
await _closeTerminalSessionIfNeeded(tabKey, persistAll: true);
} catch (e) {
debugPrint('[TerminalTabPage] Session cleanup failed for $tabKey: $e');
}
}).toList();
if (futures.isNotEmpty) {
await Future.wait(futures).timeout(
const Duration(seconds: 4),
onTimeout: () {
debugPrint(
'[TerminalTabPage] Session cleanup timed out for batch close');
return [];
},
);
}
}

/// Close the terminal session on server side based on persistent mode.
///
/// [persistAll] controls behavior when persistent mode is enabled:
/// - `true` (window close): persist all sessions, don't close any.
/// - `false` (tab close): only persist the last session for the peer,
/// close others so only the most recent disconnected session survives.
Future<void> _closeTerminalSessionIfNeeded(String tabKey,
{bool persistAll = false, int? peerTabCount}) async {
final parsed = _parseTabKey(tabKey);
if (parsed == null) return;
final (peerId, terminalId) = parsed;

final ffi = TerminalConnectionManager.getExistingConnection(peerId);
if (ffi == null) return;

final isPersistent = bind.sessionGetToggleOptionSync(
sessionId: ffi.sessionId,
arg: kOptionTerminalPersistent,
);

if (isPersistent) {
if (persistAll) {
// Window close: persist all sessions
return;
}
// Tab close: only persist if this is the last tab for this peer.
// Use the snapshot value if provided (avoids race with concurrent tab removal).
final effectivePeerTabCount = peerTabCount ??
tabController.state.value.tabs.where((t) {
final p = _parseTabKey(t.key);
return p != null && p.$1 == peerId;
}).length;
if (effectivePeerTabCount <= 1) {
// Last tab for this peer — persist the session
return;
}
// Not the last tab — fall through to close the session
}

final terminalModel = ffi.terminalModels[terminalId];
if (terminalModel != null) {
// closeTerminal() has internal 3s timeout, no need for external timeout
await terminalModel.closeTerminal();
}
}

/// Parse tabKey (format: "peerId_terminalId") into its components.
/// Note: peerId may contain underscores, so we use lastIndexOf('_').
/// Returns null if tabKey format is invalid.
(String peerId, int terminalId)? _parseTabKey(String tabKey) {
final lastUnderscore = tabKey.lastIndexOf('_');
if (lastUnderscore <= 0) {
debugPrint('[TerminalTabPage] Invalid tabKey format: $tabKey');
return null;
}
final terminalIdStr = tabKey.substring(lastUnderscore + 1);
final terminalId = int.tryParse(terminalIdStr);
if (terminalId == null) {
debugPrint('[TerminalTabPage] Invalid terminalId in tabKey: $tabKey');
return null;
}
final peerId = tabKey.substring(0, lastUnderscore);
return (peerId, terminalId);
}

Widget _tabMenuBuilder(String peerId, CancelFunc cancelFunc) {
final List<MenuEntryBase<String>> menu = [];
const EdgeInsets padding = EdgeInsets.only(left: 8.0, right: 5.0);
Expand Down Expand Up @@ -185,7 +313,8 @@ class _TerminalTabPageState extends State<TerminalTabPage> {
} else if (call.method == kWindowEventRestoreTerminalSessions) {
_restoreSessions(call.arguments);
} else if (call.method == "onDestroy") {
tabController.clear();
// Clean up sessions before window destruction (bounded wait)
await _closeAllTabs();
} else if (call.method == kWindowActionRebuild) {
reloadCurrentWindow();
} else if (call.method == kWindowEventActiveSession) {
Expand Down Expand Up @@ -269,7 +398,7 @@ class _TerminalTabPageState extends State<TerminalTabPage> {
// macOS: Cmd+W (standard for close tab)
final currentTab = tabController.state.value.selectedTabInfo;
if (tabController.state.value.tabs.length > 1) {
tabController.closeBy(currentTab.key);
_closeTab(currentTab.key);
return true;
}
} else if (!isMacOS &&
Expand All @@ -278,7 +407,7 @@ class _TerminalTabPageState extends State<TerminalTabPage> {
// Other platforms: Ctrl+Shift+W (to avoid conflict with Ctrl+W word delete)
final currentTab = tabController.state.value.selectedTabInfo;
if (tabController.state.value.tabs.length > 1) {
tabController.closeBy(currentTab.key);
_closeTab(currentTab.key);
return true;
}
}
Expand Down Expand Up @@ -357,12 +486,10 @@ class _TerminalTabPageState extends State<TerminalTabPage> {

void _addNewTerminalForCurrentPeer({int? terminalId}) {
final currentTab = tabController.state.value.selectedTabInfo;
final tabKey = currentTab.key;
final lastUnderscore = tabKey.lastIndexOf('_');
if (lastUnderscore > 0) {
final peerId = tabKey.substring(0, lastUnderscore);
_addNewTerminal(peerId, terminalId: terminalId);
}
final parsed = _parseTabKey(currentTab.key);
if (parsed == null) return;
final (peerId, _) = parsed;
_addNewTerminal(peerId, terminalId: terminalId);
}

@override
Expand All @@ -376,11 +503,9 @@ class _TerminalTabPageState extends State<TerminalTabPage> {
selectedBorderColor: MyTheme.accent,
labelGetter: DesktopTab.tablabelGetter,
tabMenuBuilder: (key) {
// Extract peerId from tab key (format: "peerId_terminalId")
// Use lastIndexOf to handle peerIds containing underscores
final lastUnderscore = key.lastIndexOf('_');
if (lastUnderscore <= 0) return Container();
final peerId = key.substring(0, lastUnderscore);
final parsed = _parseTabKey(key);
if (parsed == null) return Container();
final (peerId, _) = parsed;
return _tabMenuBuilder(peerId, () {});
},
));
Expand Down Expand Up @@ -435,7 +560,7 @@ class _TerminalTabPageState extends State<TerminalTabPage> {
}
}
if (connLength <= 1) {
tabController.clear();
await _closeAllTabs();
return true;
} else {
final bool res;
Expand All @@ -446,7 +571,7 @@ class _TerminalTabPageState extends State<TerminalTabPage> {
res = await closeConfirmDialog();
}
if (res) {
tabController.clear();
await _closeAllTabs();
}
return res;
}
Expand Down
28 changes: 27 additions & 1 deletion src/server/terminal_service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -777,6 +777,32 @@ impl TerminalServiceProxy {
) -> Result<Option<TerminalResponse>> {
let mut response = TerminalResponse::new();

// When the client requests a terminal_id that doesn't exist but there are
// surviving persistent sessions, remap the lowest-ID session to the requested
// terminal_id. This handles the case where _nextTerminalId resets to 1 on
// reconnect but the server-side sessions have non-contiguous IDs (e.g. {2: htop}).
//
// The client's requested terminal_id may not match any surviving session ID
// (e.g. _nextTerminalId incremented beyond the surviving IDs). This remap is a
// one-time handle reassignment — only the first reconnect triggers it because
// needs_session_sync is cleared afterward. Remaining sessions are communicated
// back via `persistent_sessions` with their original server-side IDs.
if !service.sessions.contains_key(&open.terminal_id)
&& service.needs_session_sync
&& !service.sessions.is_empty()
{
if let Some(&lowest_id) = service.sessions.keys().min() {
log::info!(
"Remapping persistent session {} -> {} for reconnection",
lowest_id,
open.terminal_id
);
if let Some(session_arc) = service.sessions.remove(&lowest_id) {
service.sessions.insert(open.terminal_id, session_arc);
}
}
}

// Check if terminal already exists
if let Some(session_arc) = service.sessions.get(&open.terminal_id) {
// Reconnect to existing terminal
Expand Down Expand Up @@ -824,7 +850,7 @@ impl TerminalServiceProxy {

// Create new terminal session
log::info!(
"Creating new terminal {} for service: {}",
"Creating new terminal {} for service {}",
open.terminal_id,
service.service_id
);
Expand Down
Loading