fix(terminal): prevent double session creation, restore sessions on reload
Three fixes for multi-session terminal bugs: 1. Race-condition double creation: The auto-create effect fired twice because loadSessions returned 0 while an earlier createSession was still in flight. Added hasAutoCreated guard ref to ensure only one auto-create happens. 2. Page reload spawns new sessions: After server restart, list_terminal_sessions filtered out DB-only sessions (no in-memory counterpart), so the frontend thought no sessions existed and auto-created new ones. Reverted the filter so DB rows are always returned. The WebSocket handler now restores the in-memory session from the DB row on demand when connecting. 3. No input after connection: The backend _read_loop would break on any send error, causing asyncio.wait to cancel the _write_loop. Made _read_loop retry up to 3 times before giving up, preventing transient send errors from killing input handling. Quality gates: pytest (15/15 passed), tsc clean
This commit is contained in:
@@ -153,13 +153,36 @@ async def _handle_terminal_websocket(
|
|||||||
target_session_id,
|
target_session_id,
|
||||||
)
|
)
|
||||||
if session is None:
|
if session is None:
|
||||||
logger.warning(
|
# Session not in memory — may have been lost on server restart.
|
||||||
"Session %s not found for instance %s",
|
# Try to restore from the DB row.
|
||||||
target_session_id,
|
db_row = await db_session.get(
|
||||||
instance_id,
|
TerminalSessionModel, uuid.UUID(target_session_id)
|
||||||
)
|
)
|
||||||
await websocket.close(code=4004, reason="Session not found")
|
if (
|
||||||
return
|
db_row is not None
|
||||||
|
and db_row.instance_id == instance_uuid
|
||||||
|
and db_row.status != "closed"
|
||||||
|
):
|
||||||
|
logger.info(
|
||||||
|
"Restoring terminal session %s for instance %s from DB",
|
||||||
|
target_session_id,
|
||||||
|
instance_id,
|
||||||
|
)
|
||||||
|
session = await terminal_manager.create_session(
|
||||||
|
instance_uuid,
|
||||||
|
instance.container_id,
|
||||||
|
startup_command=startup_command,
|
||||||
|
name=db_row.name,
|
||||||
|
session_id=target_session_id,
|
||||||
|
)
|
||||||
|
else:
|
||||||
|
logger.warning(
|
||||||
|
"Session %s not found for instance %s",
|
||||||
|
target_session_id,
|
||||||
|
instance_id,
|
||||||
|
)
|
||||||
|
await websocket.close(code=4004, reason="Session not found")
|
||||||
|
return
|
||||||
# Determine slot key for reset scoping
|
# Determine slot key for reset scoping
|
||||||
key = terminal_manager._find_key_by_internal_id(
|
key = terminal_manager._find_key_by_internal_id(
|
||||||
instance_id, session.session_id
|
instance_id, session.session_id
|
||||||
@@ -231,6 +254,7 @@ async def _handle_terminal_websocket(
|
|||||||
|
|
||||||
async def _read_loop(session_ref: SessionRef, websocket) -> None:
|
async def _read_loop(session_ref: SessionRef, websocket) -> None:
|
||||||
"""Read output from the container and send to WebSocket."""
|
"""Read output from the container and send to WebSocket."""
|
||||||
|
send_failures = 0
|
||||||
try:
|
try:
|
||||||
while True:
|
while True:
|
||||||
session = session_ref.session
|
session = session_ref.session
|
||||||
@@ -241,8 +265,15 @@ async def _read_loop(session_ref: SessionRef, websocket) -> None:
|
|||||||
if data:
|
if data:
|
||||||
try:
|
try:
|
||||||
await websocket.send_bytes(data)
|
await websocket.send_bytes(data)
|
||||||
|
send_failures = 0
|
||||||
except Exception:
|
except Exception:
|
||||||
break
|
# Send failed — client may have disconnected.
|
||||||
|
# Allow a few retries before giving up so transient
|
||||||
|
# errors don't kill the whole session.
|
||||||
|
send_failures += 1
|
||||||
|
if send_failures >= 3:
|
||||||
|
break
|
||||||
|
await asyncio.sleep(0.1)
|
||||||
else:
|
else:
|
||||||
await asyncio.sleep(0.01)
|
await asyncio.sleep(0.01)
|
||||||
except Exception:
|
except Exception:
|
||||||
@@ -408,19 +439,19 @@ async def list_terminal_sessions(
|
|||||||
db_rows = result.scalars().all()
|
db_rows = result.scalars().all()
|
||||||
|
|
||||||
# Build response with live has_websockets flag.
|
# Build response with live has_websockets flag.
|
||||||
# Skip DB-only sessions that have no live in-memory counterpart
|
# Include DB rows even without in-memory counterparts (e.g. after
|
||||||
# (e.g. after server restart).
|
# server restart) so the frontend can display tabs and reconnect.
|
||||||
sessions = []
|
sessions = []
|
||||||
for row in db_rows:
|
for row in db_rows:
|
||||||
live_session = terminal_manager.get_session(str(instance_id), str(row.id))
|
live_session = terminal_manager.get_session(str(instance_id), str(row.id))
|
||||||
if not live_session:
|
|
||||||
continue
|
|
||||||
sessions.append(
|
sessions.append(
|
||||||
{
|
{
|
||||||
"id": str(row.id),
|
"id": str(row.id),
|
||||||
"name": row.name,
|
"name": row.name,
|
||||||
"status": row.status,
|
"status": row.status,
|
||||||
"has_websockets": live_session.has_websockets(),
|
"has_websockets": live_session.has_websockets()
|
||||||
|
if live_session
|
||||||
|
else False,
|
||||||
"created_at": row.created_at.isoformat() if row.created_at else None,
|
"created_at": row.created_at.isoformat() if row.created_at else None,
|
||||||
"last_activity_at": row.last_activity_at.isoformat()
|
"last_activity_at": row.last_activity_at.isoformat()
|
||||||
if row.last_activity_at
|
if row.last_activity_at
|
||||||
|
|||||||
@@ -131,6 +131,7 @@ class TerminalManager:
|
|||||||
container_id: str,
|
container_id: str,
|
||||||
startup_command: str | None = None,
|
startup_command: str | None = None,
|
||||||
name: str | None = None,
|
name: str | None = None,
|
||||||
|
session_id: str | None = None,
|
||||||
) -> TerminalSession:
|
) -> TerminalSession:
|
||||||
"""Create a new terminal session for an instance.
|
"""Create a new terminal session for an instance.
|
||||||
|
|
||||||
@@ -159,7 +160,8 @@ class TerminalManager:
|
|||||||
instance_id_str, self.MAX_SESSIONS_PER_INSTANCE
|
instance_id_str, self.MAX_SESSIONS_PER_INSTANCE
|
||||||
)
|
)
|
||||||
|
|
||||||
session_id = str(uuid.uuid4())
|
if session_id is None:
|
||||||
|
session_id = str(uuid.uuid4())
|
||||||
session = TerminalSession(
|
session = TerminalSession(
|
||||||
session_id=session_id,
|
session_id=session_id,
|
||||||
instance_id=instance_id,
|
instance_id=instance_id,
|
||||||
@@ -172,7 +174,7 @@ class TerminalManager:
|
|||||||
key = (instance_id_str, session_id)
|
key = (instance_id_str, session_id)
|
||||||
self._sessions[key] = session
|
self._sessions[key] = session
|
||||||
|
|
||||||
# Fire-and-forget DB insert
|
# Fire-and-forget DB insert (skip if row already exists)
|
||||||
asyncio.create_task(
|
asyncio.create_task(
|
||||||
self._insert_db_session_row(session_id, instance_id, session.name)
|
self._insert_db_session_row(session_id, instance_id, session.name)
|
||||||
)
|
)
|
||||||
|
|||||||
@@ -26,6 +26,7 @@ export const TerminalPage: React.FC = () => {
|
|||||||
const [isFullscreen, setIsFullscreen] = useState(false);
|
const [isFullscreen, setIsFullscreen] = useState(false);
|
||||||
const terminalRefs = useRef<Record<string, React.RefObject<TerminalRef>>>({});
|
const terminalRefs = useRef<Record<string, React.RefObject<TerminalRef>>>({});
|
||||||
const headerAutoHide = useAutoHide({ timeout: 3000, enabled: isMobile });
|
const headerAutoHide = useAutoHide({ timeout: 3000, enabled: isMobile });
|
||||||
|
const hasAutoCreated = useRef(false);
|
||||||
|
|
||||||
const {
|
const {
|
||||||
sessions,
|
sessions,
|
||||||
@@ -39,9 +40,16 @@ export const TerminalPage: React.FC = () => {
|
|||||||
error,
|
error,
|
||||||
} = useTerminalSessions(instanceId ?? "");
|
} = useTerminalSessions(instanceId ?? "");
|
||||||
|
|
||||||
// Auto-create default session if none exist
|
// Auto-create default session if none exist (guard against double-fire)
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
if (!loading && sessions.length === 0 && !error && instanceId) {
|
if (
|
||||||
|
!loading &&
|
||||||
|
sessions.length === 0 &&
|
||||||
|
!error &&
|
||||||
|
instanceId &&
|
||||||
|
!hasAutoCreated.current
|
||||||
|
) {
|
||||||
|
hasAutoCreated.current = true;
|
||||||
void createSession("Session 1");
|
void createSession("Session 1");
|
||||||
}
|
}
|
||||||
}, [loading, sessions.length, error, instanceId, createSession]);
|
}, [loading, sessions.length, error, instanceId, createSession]);
|
||||||
@@ -232,16 +240,13 @@ export const TerminalPage: React.FC = () => {
|
|||||||
{sessions
|
{sessions
|
||||||
.filter((session) => session.id === activeSessionId)
|
.filter((session) => session.id === activeSessionId)
|
||||||
.map((session) => (
|
.map((session) => (
|
||||||
<div
|
<div key={session.id} className="terminal-instance active">
|
||||||
key={session.id}
|
|
||||||
className="terminal-instance active"
|
|
||||||
>
|
|
||||||
<TerminalComponent
|
<TerminalComponent
|
||||||
ref={terminalRefs.current[session.id]}
|
ref={terminalRefs.current[session.id]}
|
||||||
instanceId={instanceId}
|
instanceId={instanceId}
|
||||||
sessionId={session.id}
|
sessionId={session.id}
|
||||||
onClose={() => handleClose(session.id)}
|
onClose={() => handleClose(session.id)}
|
||||||
isMobile={true}
|
isMobile={true}
|
||||||
/>
|
/>
|
||||||
</div>
|
</div>
|
||||||
))}
|
))}
|
||||||
@@ -291,10 +296,7 @@ export const TerminalPage: React.FC = () => {
|
|||||||
{sessions
|
{sessions
|
||||||
.filter((session) => session.id === activeSessionId)
|
.filter((session) => session.id === activeSessionId)
|
||||||
.map((session) => (
|
.map((session) => (
|
||||||
<div
|
<div key={session.id} className="terminal-instance active">
|
||||||
key={session.id}
|
|
||||||
className="terminal-instance active"
|
|
||||||
>
|
|
||||||
<TerminalComponent
|
<TerminalComponent
|
||||||
ref={terminalRefs.current[session.id]}
|
ref={terminalRefs.current[session.id]}
|
||||||
instanceId={instanceId}
|
instanceId={instanceId}
|
||||||
|
|||||||
Reference in New Issue
Block a user