Skip to content

Commit a977208

Browse files
committed
refactor: improve port cleanup with graceful termination and fallbacks
- Add isProcessAlive() helper to check process status cross-platform - Implement graceful kill on Unix (SIGTERM → SIGKILL escalation) - Implement graceful kill on Windows (taskkill → taskkill /F escalation) - Deduplicate PIDs before processing to avoid double-kills - Add ss fallback when lsof is unavailable on Unix - Filter for LISTEN sockets only using lsof -sTCP:LISTEN - Wait 1 second after SIGTERM before escalating to SIGKILL - Improve error handling and logging throughout cleanup process
1 parent cbcbca8 commit a977208

File tree

1 file changed

+146
-33
lines changed

1 file changed

+146
-33
lines changed

src/kernels/deepnote/deepnoteServerStarter.node.ts

Lines changed: 146 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -644,6 +644,60 @@ export class DeepnoteServerStarter implements IDeepnoteServerStarter, IExtension
644644
return false;
645645
}
646646

647+
/**
648+
* Check if a process is still alive.
649+
*/
650+
private async isProcessAlive(pid: number): Promise<boolean> {
651+
try {
652+
const processService = await this.processServiceFactory.create(undefined);
653+
if (process.platform === 'win32') {
654+
const result = await processService.exec('tasklist', ['/FI', `PID eq ${pid}`, '/NH'], {
655+
throwOnStdErr: false
656+
});
657+
return result.stdout.includes(pid.toString());
658+
} else {
659+
// Use kill -0 to check if process exists (doesn't actually kill)
660+
// If it succeeds, process exists; if it fails, process doesn't exist
661+
try {
662+
await processService.exec('kill', ['-0', pid.toString()], { throwOnStdErr: false });
663+
return true;
664+
} catch {
665+
return false;
666+
}
667+
}
668+
} catch {
669+
return false;
670+
}
671+
}
672+
673+
/**
674+
* Attempt graceful kill (SIGTERM) then escalate to SIGKILL if needed.
675+
*/
676+
private async killProcessGracefully(
677+
pid: number,
678+
processService: import('../../platform/common/process/types.node').IProcessService
679+
): Promise<void> {
680+
try {
681+
// Try graceful termination first (SIGTERM)
682+
logger.debug(`Attempting graceful termination of process ${pid} (SIGTERM)...`);
683+
await processService.exec('kill', [pid.toString()], { throwOnStdErr: false });
684+
685+
// Wait a bit for graceful shutdown
686+
await new Promise((resolve) => setTimeout(resolve, 1000));
687+
688+
// Check if still alive
689+
const stillAlive = await this.isProcessAlive(pid);
690+
if (stillAlive) {
691+
logger.debug(`Process ${pid} did not terminate gracefully, escalating to SIGKILL...`);
692+
await processService.exec('kill', ['-9', pid.toString()], { throwOnStdErr: false });
693+
} else {
694+
logger.debug(`Process ${pid} terminated gracefully`);
695+
}
696+
} catch (ex) {
697+
logger.debug(`Error during graceful kill of process ${pid}: ${ex}`);
698+
}
699+
}
700+
647701
/**
648702
* Find and kill orphaned deepnote-toolkit processes using specific ports.
649703
* This is useful for cleaning up LSP servers and Jupyter servers that may be stuck.
@@ -654,46 +708,25 @@ export class DeepnoteServerStarter implements IDeepnoteServerStarter, IExtension
654708
const processService = await this.processServiceFactory.create(undefined);
655709

656710
if (process.platform === 'win32') {
657-
// Windows: use netstat to find process using port
711+
// Windows: use netstat to find LISTENING processes on port
658712
const result = await processService.exec('netstat', ['-ano'], { throwOnStdErr: false });
659713
if (result.stdout) {
660714
const lines = result.stdout.split('\n');
715+
const uniquePids = new Set<number>();
716+
717+
// Parse and deduplicate PIDs
661718
for (const line of lines) {
662719
if (line.includes(`:${port}`) && line.includes('LISTENING')) {
663720
const parts = line.trim().split(/\s+/);
664721
const pid = parseInt(parts[parts.length - 1], 10);
665722
if (!isNaN(pid) && pid > 0) {
666-
// Check if it's deepnote-related first
667-
const isDeepnoteRelated = await this.isDeepnoteRelatedProcess(pid);
668-
if (!isDeepnoteRelated) {
669-
logger.debug(`Process ${pid} on port ${port} is not deepnote-related, skipping`);
670-
continue;
671-
}
672-
673-
const isOrphaned = await this.isProcessOrphaned(pid);
674-
if (isOrphaned) {
675-
logger.info(
676-
`Found orphaned deepnote-related process ${pid} using port ${port}, killing...`
677-
);
678-
await processService.exec('taskkill', ['/F', '/PID', pid.toString()], {
679-
throwOnStdErr: false
680-
});
681-
}
723+
uniquePids.add(pid);
682724
}
683725
}
684726
}
685-
}
686-
} else {
687-
// Unix-like: use lsof to find process using port
688-
const result = await processService.exec('lsof', ['-i', `:${port}`, '-t'], { throwOnStdErr: false });
689-
if (result.stdout) {
690-
const pids = result.stdout
691-
.trim()
692-
.split('\n')
693-
.map((p) => parseInt(p.trim(), 10))
694-
.filter((p) => !isNaN(p) && p > 0);
695727

696-
for (const pid of pids) {
728+
// Process each unique PID
729+
for (const pid of uniquePids) {
697730
// Check if it's deepnote-related first
698731
const isDeepnoteRelated = await this.isDeepnoteRelatedProcess(pid);
699732
if (!isDeepnoteRelated) {
@@ -704,16 +737,96 @@ export class DeepnoteServerStarter implements IDeepnoteServerStarter, IExtension
704737
const isOrphaned = await this.isProcessOrphaned(pid);
705738
if (isOrphaned) {
706739
logger.info(
707-
`Found orphaned deepnote-related process ${pid} using port ${port}, killing...`
740+
`Found orphaned deepnote-related process ${pid} using port ${port}, killing process tree...`
708741
);
709-
await processService.exec('kill', ['-9', pid.toString()], { throwOnStdErr: false });
742+
// Try without /F first (graceful)
743+
try {
744+
await processService.exec('taskkill', ['/T', '/PID', pid.toString()], {
745+
throwOnStdErr: false
746+
});
747+
logger.debug(`Gracefully killed process ${pid}`);
748+
} catch (gracefulError) {
749+
// If graceful kill failed, use /F (force)
750+
logger.debug(`Graceful kill failed for ${pid}, using /F flag...`);
751+
try {
752+
await processService.exec('taskkill', ['/F', '/T', '/PID', pid.toString()], {
753+
throwOnStdErr: false
754+
});
755+
} catch (forceError) {
756+
logger.debug(`Force kill also failed for ${pid}: ${forceError}`);
757+
}
758+
}
710759
} else {
711-
logger.info(
712-
`Deepnote-related process ${pid} using port ${port} has active parent, skipping`
713-
);
760+
logger.debug(`Deepnote-related process ${pid} on port ${port} has active parent, skipping`);
714761
}
715762
}
716763
}
764+
} else {
765+
// Unix-like: try lsof first, fallback to ss
766+
let uniquePids = new Set<number>();
767+
768+
// Try lsof with LISTEN filter
769+
try {
770+
const lsofResult = await processService.exec('lsof', ['-sTCP:LISTEN', '-i', `:${port}`, '-t'], {
771+
throwOnStdErr: false
772+
});
773+
if (lsofResult.stdout) {
774+
const pids = lsofResult.stdout
775+
.trim()
776+
.split('\n')
777+
.map((p) => parseInt(p.trim(), 10))
778+
.filter((p) => !isNaN(p) && p > 0);
779+
pids.forEach((pid) => uniquePids.add(pid));
780+
}
781+
} catch (lsofError) {
782+
logger.debug(`lsof failed or unavailable, trying ss: ${lsofError}`);
783+
}
784+
785+
// Fallback to ss if lsof didn't find anything or failed
786+
if (uniquePids.size === 0) {
787+
try {
788+
const ssResult = await processService.exec('ss', ['-tlnp', `sport = :${port}`], {
789+
throwOnStdErr: false
790+
});
791+
if (ssResult.stdout) {
792+
// Parse ss output: look for pid=<number>
793+
const pidMatches = ssResult.stdout.matchAll(/pid=(\d+)/g);
794+
for (const match of pidMatches) {
795+
const pid = parseInt(match[1], 10);
796+
if (!isNaN(pid) && pid > 0) {
797+
uniquePids.add(pid);
798+
}
799+
}
800+
}
801+
} catch (ssError) {
802+
logger.debug(`ss also failed: ${ssError}`);
803+
}
804+
}
805+
806+
if (uniquePids.size === 0) {
807+
logger.debug(`No processes found listening on port ${port}`);
808+
return;
809+
}
810+
811+
// Process each unique PID
812+
for (const pid of uniquePids) {
813+
// Check if it's deepnote-related first
814+
const isDeepnoteRelated = await this.isDeepnoteRelatedProcess(pid);
815+
if (!isDeepnoteRelated) {
816+
logger.debug(`Process ${pid} on port ${port} is not deepnote-related, skipping`);
817+
continue;
818+
}
819+
820+
const isOrphaned = await this.isProcessOrphaned(pid);
821+
if (isOrphaned) {
822+
logger.info(
823+
`Found orphaned deepnote-related process ${pid} using port ${port}, attempting graceful kill...`
824+
);
825+
await this.killProcessGracefully(pid, processService);
826+
} else {
827+
logger.debug(`Deepnote-related process ${pid} on port ${port} has active parent, skipping`);
828+
}
829+
}
717830
}
718831
} catch (ex) {
719832
logger.debug(`Failed to cleanup processes on port ${port}: ${ex}`);

0 commit comments

Comments
 (0)