Skip to content

Commit 887834e

Browse files
jankucajankuca
committed
fix: improve orphaned process cleanup to handle child processes safely
- Add port-based cleanup for LSP (2087) and Jupyter (8888) servers - Add isDeepnoteRelatedProcess() to verify processes are from deepnote-venvs - Expand process detection to include pylsp and jupyter child processes - Prevent killing external Jupyter/LSP servers not managed by extension - Fix 'Address already in use' errors from orphaned LSP servers The cleanup now uses a two-stage safety check: 1. Verify process is deepnote-related (from deepnote-venvs directory) 2. Verify process is orphaned (parent no longer exists) This prevents interference with user's manual Jupyter servers or other tools while still cleaning up stuck processes from crashed VSCode sessions.
1 parent e2fa9b5 commit 887834e

File tree

1 file changed

+140
-11
lines changed

1 file changed

+140
-11
lines changed

‎src/kernels/deepnote/deepnoteServerStarter.node.ts‎

Lines changed: 140 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -589,16 +589,134 @@ export class DeepnoteServerStarter implements IDeepnoteServerStarter, IExtension
589589
return false;
590590
}
591591

592+
/**
593+
* Check if a process is a deepnote-toolkit related process by examining its command line.
594+
*/
595+
private async isDeepnoteRelatedProcess(pid: number): Promise<boolean> {
596+
try {
597+
const processService = await this.processServiceFactory.create(undefined);
598+
599+
if (process.platform === 'win32') {
600+
// Windows: use wmic to get command line
601+
const result = await processService.exec(
602+
'wmic',
603+
['process', 'where', `ProcessId=${pid}`, 'get', 'CommandLine'],
604+
{ throwOnStdErr: false }
605+
);
606+
if (result.stdout) {
607+
const cmdLine = result.stdout.toLowerCase();
608+
// Check if it's running from our deepnote-venvs directory or is deepnote_toolkit
609+
return cmdLine.includes('deepnote-venvs') || cmdLine.includes('deepnote_toolkit');
610+
}
611+
} else {
612+
// Unix-like: use ps to get command line
613+
const result = await processService.exec('ps', ['-p', pid.toString(), '-o', 'command='], {
614+
throwOnStdErr: false
615+
});
616+
if (result.stdout) {
617+
const cmdLine = result.stdout.toLowerCase();
618+
// Check if it's running from our deepnote-venvs directory or is deepnote_toolkit
619+
return cmdLine.includes('deepnote-venvs') || cmdLine.includes('deepnote_toolkit');
620+
}
621+
}
622+
} catch (ex) {
623+
logger.debug(`Failed to check if process ${pid} is deepnote-related: ${ex}`);
624+
}
625+
return false;
626+
}
627+
628+
/**
629+
* Find and kill orphaned deepnote-toolkit processes using specific ports.
630+
* This is useful for cleaning up LSP servers and Jupyter servers that may be stuck.
631+
* Only kills processes that are both orphaned AND deepnote-related.
632+
*/
633+
private async cleanupProcessesByPort(port: number): Promise<void> {
634+
try {
635+
const processService = await this.processServiceFactory.create(undefined);
636+
637+
if (process.platform === 'win32') {
638+
// Windows: use netstat to find process using port
639+
const result = await processService.exec('netstat', ['-ano'], { throwOnStdErr: false });
640+
if (result.stdout) {
641+
const lines = result.stdout.split('\n');
642+
for (const line of lines) {
643+
if (line.includes(`:${port}`) && line.includes('LISTENING')) {
644+
const parts = line.trim().split(/\s+/);
645+
const pid = parseInt(parts[parts.length - 1], 10);
646+
if (!isNaN(pid) && pid > 0) {
647+
// Check if it's deepnote-related first
648+
const isDeepnoteRelated = await this.isDeepnoteRelatedProcess(pid);
649+
if (!isDeepnoteRelated) {
650+
logger.debug(`Process ${pid} on port ${port} is not deepnote-related, skipping`);
651+
continue;
652+
}
653+
654+
const isOrphaned = await this.isProcessOrphaned(pid);
655+
if (isOrphaned) {
656+
logger.info(
657+
`Found orphaned deepnote-related process ${pid} using port ${port}, killing...`
658+
);
659+
await processService.exec('taskkill', ['/F', '/PID', pid.toString()], {
660+
throwOnStdErr: false
661+
});
662+
}
663+
}
664+
}
665+
}
666+
}
667+
} else {
668+
// Unix-like: use lsof to find process using port
669+
const result = await processService.exec('lsof', ['-i', `:${port}`, '-t'], { throwOnStdErr: false });
670+
if (result.stdout) {
671+
const pids = result.stdout
672+
.trim()
673+
.split('\n')
674+
.map((p) => parseInt(p.trim(), 10))
675+
.filter((p) => !isNaN(p) && p > 0);
676+
677+
for (const pid of pids) {
678+
// Check if it's deepnote-related first
679+
const isDeepnoteRelated = await this.isDeepnoteRelatedProcess(pid);
680+
if (!isDeepnoteRelated) {
681+
logger.debug(`Process ${pid} on port ${port} is not deepnote-related, skipping`);
682+
continue;
683+
}
684+
685+
const isOrphaned = await this.isProcessOrphaned(pid);
686+
if (isOrphaned) {
687+
logger.info(
688+
`Found orphaned deepnote-related process ${pid} using port ${port}, killing...`
689+
);
690+
await processService.exec('kill', ['-9', pid.toString()], { throwOnStdErr: false });
691+
} else {
692+
logger.info(
693+
`Deepnote-related process ${pid} using port ${port} has active parent, skipping`
694+
);
695+
}
696+
}
697+
}
698+
}
699+
} catch (ex) {
700+
logger.debug(`Failed to cleanup processes on port ${port}: ${ex}`);
701+
}
702+
}
703+
592704
/**
593705
* Cleans up any orphaned deepnote-toolkit processes from previous VS Code sessions.
594706
* This prevents port conflicts when starting new servers.
595707
*/
596708
private async cleanupOrphanedProcesses(): Promise<void> {
597709
try {
598710
logger.info('Checking for orphaned deepnote-toolkit processes...');
711+
712+
// First, clean up any orphaned processes using known ports
713+
// This catches LSP servers (2087) and Jupyter servers (8888+) that may be stuck
714+
await this.cleanupProcessesByPort(2087); // Python LSP server
715+
await this.cleanupProcessesByPort(8888); // Default Jupyter port
716+
599717
const processService = await this.processServiceFactory.create(undefined);
600718

601-
// Find all deepnote-toolkit server processes
719+
// Find all deepnote-toolkit server processes and related child processes
602720
let command: string;
603721
let args: string[];
604722

@@ -619,8 +737,17 @@ export class DeepnoteServerStarter implements IDeepnoteServerStarter, IExtension
619737
const candidatePids: number[] = [];
620738

621739
for (const line of lines) {
622-
// Look for processes running deepnote_toolkit server
623-
if (line.includes('deepnote_toolkit') && line.includes('server')) {
740+
// Look for processes running deepnote_toolkit server or related child processes
741+
// This includes:
742+
// - deepnote_toolkit server (main server process)
743+
// - pylsp (Python LSP server child process)
744+
// - jupyter (Jupyter server child process)
745+
const isDeepnoteRelated =
746+
(line.includes('deepnote_toolkit') && line.includes('server')) ||
747+
(line.includes('pylsp') && line.includes('2087')) || // LSP server on port 2087
748+
(line.includes('jupyter') && line.includes('deepnote'));
749+
750+
if (isDeepnoteRelated) {
624751
// Extract PID based on platform
625752
let pid: number | undefined;
626753

@@ -646,19 +773,20 @@ export class DeepnoteServerStarter implements IDeepnoteServerStarter, IExtension
646773

647774
if (candidatePids.length > 0) {
648775
logger.info(
649-
`Found ${candidatePids.length} deepnote-toolkit server process(es): ${candidatePids.join(', ')}`
776+
`Found ${candidatePids.length} deepnote-related process(es): ${candidatePids.join(', ')}`
650777
);
651778

652779
const pidsToKill: number[] = [];
653780
const pidsToSkip: Array<{ pid: number; reason: string }> = [];
654781

655782
// Check each process to determine if it should be killed
656783
for (const pid of candidatePids) {
657-
// Check if there's a lock file for this PID
784+
// Check if there's a lock file for this PID (only main server processes have lock files)
658785
const lockData = await this.readLockFile(pid);
659786

660787
if (lockData) {
661-
// Lock file exists - check if it belongs to a different session
788+
// Lock file exists - this is a main server process
789+
// Check if it belongs to a different session
662790
if (lockData.sessionId !== this.sessionId) {
663791
// Different session - check if the process is actually orphaned
664792
const isOrphaned = await this.isProcessOrphaned(pid);
@@ -678,7 +806,8 @@ export class DeepnoteServerStarter implements IDeepnoteServerStarter, IExtension
678806
pidsToSkip.push({ pid, reason: 'belongs to current session' });
679807
}
680808
} else {
681-
// No lock file - check if orphaned before killing
809+
// No lock file - could be a child process (LSP, Jupyter) or orphaned main process
810+
// Check if orphaned before killing
682811
const isOrphaned = await this.isProcessOrphaned(pid);
683812
if (isOrphaned) {
684813
logger.info(`PID ${pid} has no lock file and is orphaned - will kill`);
@@ -700,7 +829,7 @@ export class DeepnoteServerStarter implements IDeepnoteServerStarter, IExtension
700829
if (pidsToKill.length > 0) {
701830
logger.info(`Killing ${pidsToKill.length} orphaned process(es): ${pidsToKill.join(', ')}`);
702831
this.outputChannel.appendLine(
703-
`Cleaning up ${pidsToKill.length} orphaned deepnote-toolkit process(es)...`
832+
`Cleaning up ${pidsToKill.length} orphaned deepnote-related process(es)...`
704833
);
705834

706835
for (const pid of pidsToKill) {
@@ -714,7 +843,7 @@ export class DeepnoteServerStarter implements IDeepnoteServerStarter, IExtension
714843
}
715844
logger.info(`Killed orphaned process ${pid}`);
716845

717-
// Clean up the lock file after killing
846+
// Clean up the lock file after killing (if it exists)
718847
await this.deleteLockFile(pid);
719848
} catch (ex) {
720849
logger.warn(`Failed to kill process ${pid}: ${ex}`);
@@ -723,10 +852,10 @@ export class DeepnoteServerStarter implements IDeepnoteServerStarter, IExtension
723852

724853
this.outputChannel.appendLine('✓ Cleanup complete');
725854
} else {
726-
logger.info('No orphaned deepnote-toolkit processes found (all processes are active)');
855+
logger.info('No orphaned deepnote-related processes found (all processes are active)');
727856
}
728857
} else {
729-
logger.info('No deepnote-toolkit server processes found');
858+
logger.info('No deepnote-related processes found');
730859
}
731860
}
732861
} catch (ex) {

0 commit comments

Comments
 (0)