Skip to content

Commit 3754f9b

Browse files
echobtfactorydroid
andauthored
fix(terminal): improve stream closure handling and memory leak prevention (#331)
- Add dispose() method to OutputStreamProcessor to properly release callbacks - Add isDisposed() check to prevent using disposed processor - Enhance stream closure handling with multi-level checks - Track terminal disposal state to prevent writes after cleanup - Add try-catch around buffer access during deferred processing - Fix memory leak by clearing ackCallback and pendingCallback on dispose These changes address: 1. Memory leaks from closure references in output processor 2. Crashes when writing to disposed terminal 3. Race conditions during terminal cleanup 4. Buffer access errors during disposal Co-authored-by: Droid Agent <droid@factory.ai>
1 parent b6fe91d commit 3754f9b

1 file changed

Lines changed: 84 additions & 17 deletions

File tree

cortex-gui/src/components/TerminalPanel.tsx

Lines changed: 84 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -358,6 +358,24 @@ class OutputStreamProcessor {
358358
this.pendingCallback = null;
359359
this.pendingAckBytes = 0;
360360
}
361+
362+
/**
363+
* Dispose and release all resources to prevent memory leaks
364+
* Should be called when the terminal is being destroyed
365+
*/
366+
dispose(): void {
367+
this.cancel();
368+
// Clear callbacks to prevent memory leaks from closures holding references
369+
this.ackCallback = null;
370+
this.pendingCallback = null;
371+
}
372+
373+
/**
374+
* Check if the processor has been disposed
375+
*/
376+
isDisposed(): boolean {
377+
return this.ackCallback === null && this.pendingCallback === null && this.bufferChunks.length === 0;
378+
}
361379
}
362380

363381
/**
@@ -1708,18 +1726,55 @@ export function TerminalPanel() {
17081726
resizeTerminal(terminalInfo.id, cols, rows).catch(console.error);
17091727
});
17101728

1711-
// Subscribe to terminal output with chunked processing
1729+
// Track if this terminal instance has been disposed to prevent writes after cleanup
1730+
let isTerminalDisposed = false;
1731+
1732+
// Subscribe to terminal output with robust stream closure handling
17121733
const unsubscribe = subscribeToOutput((output) => {
1734+
// Early exit if terminal has been disposed
1735+
if (isTerminalDisposed) {
1736+
return;
1737+
}
1738+
17131739
if (output.terminal_id === terminalInfo.id) {
17141740
// Write output directly to terminal - no chunking for real-time TUI responsiveness
17151741
// Guard against writing to a disposed terminal to prevent crashes
17161742
try {
1717-
// Check if terminal is still usable before writing
1718-
if (terminal.element && !terminal.element.classList.contains('disposed')) {
1719-
terminal.write(output.data);
1743+
// Multi-level check for terminal availability:
1744+
// 1. Check our local disposed flag (fastest)
1745+
// 2. Check if terminal element exists
1746+
// 3. Check if terminal is marked as disposed via class
1747+
// 4. Check if the terminal is in the instances map (hasn't been cleaned up)
1748+
if (isTerminalDisposed) {
1749+
return;
17201750
}
1751+
1752+
if (!terminal.element) {
1753+
console.debug(`[Terminal] Terminal ${terminalInfo.id} element is null, marking as disposed`);
1754+
isTerminalDisposed = true;
1755+
return;
1756+
}
1757+
1758+
if (terminal.element.classList.contains('disposed')) {
1759+
console.debug(`[Terminal] Terminal ${terminalInfo.id} is disposed, skipping output`);
1760+
isTerminalDisposed = true;
1761+
return;
1762+
}
1763+
1764+
// Check if terminal instance still exists in our map
1765+
const instance = terminalInstances.get(terminalInfo.id);
1766+
if (!instance) {
1767+
console.debug(`[Terminal] Terminal ${terminalInfo.id} instance not found, marking as disposed`);
1768+
isTerminalDisposed = true;
1769+
return;
1770+
}
1771+
1772+
// Safe to write
1773+
terminal.write(output.data);
17211774
} catch (e) {
17221775
// Terminal may be disposed or in an invalid state - this is expected during cleanup
1776+
// Mark as disposed to prevent further write attempts
1777+
isTerminalDisposed = true;
17231778
console.debug(`[Terminal] Stream write failed for ${terminalInfo.id}, terminal may be closing:`, e);
17241779
return; // Skip further processing for this output
17251780
}
@@ -1728,6 +1783,11 @@ export function TerminalPanel() {
17281783
// Use requestIdleCallback for lower priority tasks
17291784
if (quickFixEnabled() || stickyScrollSettings.enabled) {
17301785
requestAnimationFrame(() => {
1786+
// Double-check terminal is still valid before deferred processing
1787+
if (isTerminalDisposed || !terminalInstances.has(terminalInfo.id)) {
1788+
return;
1789+
}
1790+
17311791
if (quickFixEnabled()) {
17321792
setTerminalOutputs(terminalInfo.id, (prev) => {
17331793
const newOutput = (prev || "") + output.data;
@@ -1739,16 +1799,22 @@ export function TerminalPanel() {
17391799
}
17401800

17411801
if (stickyScrollSettings.enabled) {
1742-
const lines = output.data.split(/\r?\n/);
1743-
const baseLineNumber = terminal.buffer.active.baseY + terminal.buffer.active.cursorY;
1744-
lines.forEach((line, index) => {
1745-
if (line.trim()) {
1746-
processStickyScrollLine(terminalInfo.id, baseLineNumber + index, line);
1747-
}
1748-
});
1749-
const scrollLine = terminal.buffer.active.viewportY;
1750-
const totalLines = terminal.buffer.active.length;
1751-
updateTerminalScrollPosition(terminalInfo.id, scrollLine, totalLines);
1802+
// Safely access terminal buffer - may throw if disposed
1803+
try {
1804+
const lines = output.data.split(/\r?\n/);
1805+
const baseLineNumber = terminal.buffer.active.baseY + terminal.buffer.active.cursorY;
1806+
lines.forEach((line, index) => {
1807+
if (line.trim()) {
1808+
processStickyScrollLine(terminalInfo.id, baseLineNumber + index, line);
1809+
}
1810+
});
1811+
const scrollLine = terminal.buffer.active.viewportY;
1812+
const totalLines = terminal.buffer.active.length;
1813+
updateTerminalScrollPosition(terminalInfo.id, scrollLine, totalLines);
1814+
} catch (bufferError) {
1815+
// Terminal buffer may be invalid during disposal
1816+
console.debug(`[Terminal] Buffer access failed for ${terminalInfo.id}:`, bufferError);
1817+
}
17521818
}
17531819
});
17541820
}
@@ -1897,9 +1963,10 @@ export function TerminalPanel() {
18971963
}
18981964

18991965
// Clean up output processor (cancel pending flush timeouts)
1966+
// Properly dispose the output processor to prevent memory leaks
19001967
const processor = outputProcessors.get(terminalId);
19011968
if (processor) {
1902-
processor.cancel();
1969+
processor.dispose();
19031970
outputProcessors.delete(terminalId);
19041971
}
19051972

@@ -2451,9 +2518,9 @@ export function TerminalPanel() {
24512518
});
24522519
terminalInstances.clear();
24532520

2454-
// Clean up output processors (cancel pending flush timeouts)
2521+
// Clean up output processors (dispose to prevent memory leaks)
24552522
outputProcessors.forEach((processor) => {
2456-
processor.cancel();
2523+
processor.dispose();
24572524
});
24582525
outputProcessors.clear();
24592526

0 commit comments

Comments
 (0)