fix: auto-kill orphaned processes #19
Conversation
📝 WalkthroughWalkthrough
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant Starter as DeepnoteServerStarter
participant Toolkit as deepnote-toolkit (proc)
participant Registry as IAsyncDisposableRegistry
Note over Starter: Constructor
rect rgba(230,250,255,0.6)
Starter->>Registry: register(Starter) [New]
Starter->>Starter: cleanupOrphanedProcesses() [New]
end
Note over Client,Starter: Get or start server
rect rgba(240,240,255,0.6)
Client->>Starter: getOrStartServer(interpreter, uri, token) [New]
alt existing server for uri
Starter-->>Client: DeepnoteServerInfo
else no server
Starter->>Starter: track pending start [Changed]
Starter->>Toolkit: spawn/start
Toolkit-->>Starter: ready/port
Starter->>Starter: record server info
Starter-->>Client: DeepnoteServerInfo
end
end
sequenceDiagram
autonumber
actor Client
participant Starter as DeepnoteServerStarter
participant Toolkit as deepnote-toolkit (proc)
participant Registry as IAsyncDisposableRegistry
Note over Client,Starter: Stop or Dispose
rect rgba(255,245,230,0.6)
alt stop specific server
Client->>Starter: stopServer(uri)
Starter->>Starter: wait for related pending ops [Changed]
Starter->>Toolkit: graceful terminate
alt exits within timeout
Toolkit-->>Starter: exit
else timeout
Starter->>Toolkit: force kill [Changed]
end
Starter->>Starter: clear server tracking
Starter-->>Client: done
else dispose all
Client->>Starter: dispose() [Changed -> async]
Starter->>Starter: wait for all pending ops [Changed]
loop each tracked server
Starter->>Toolkit: graceful terminate
alt exits within timeout
Toolkit-->>Starter: exit
else timeout
Starter->>Toolkit: force kill
end
end
Starter->>Starter: dispose internal resources
Starter-->>Client: Promise<void> resolved
end
end
Pre-merge checks✅ Passed checks (3 passed)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
src/kernels/deepnote/deepnoteServerStarter.node.ts(4 hunks)src/kernels/deepnote/types.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.node.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use
*.node.tsfor Desktop-specific implementations that require full file system access and Python environments
Files:
src/kernels/deepnote/deepnoteServerStarter.node.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Inject interfaces, not concrete classes
Avoid circular dependencies
Usel10n.t()for user-facing strings
Use typed error classes fromsrc/platform/errors/when throwing or handling errors
Use theILoggerservice instead ofconsole.log
Preserve error details while scrubbing PII in messages and telemetry
Include the Microsoft copyright header in source files
Prefer async/await and handle cancellation withCancellationToken
**/*.{ts,tsx}: Order class members (methods, fields, properties) first by accessibility, then alphabetically
Separate third-party imports from local file imports
Files:
src/kernels/deepnote/deepnoteServerStarter.node.tssrc/kernels/deepnote/types.ts
src/kernels/**/*.ts
📄 CodeRabbit inference engine (.github/instructions/kernel.instructions.md)
src/kernels/**/*.ts: Use event-driven updates (EventEmitter) for state changes
Monitor and dispose pending promises to prevent leaks during teardown
Use WeakRef/weak references for notebook/object backreferences to avoid memory leaks
Respect CancellationToken in all async operations
Files:
src/kernels/deepnote/deepnoteServerStarter.node.tssrc/kernels/deepnote/types.ts
**/!(*.node|*.web).ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Place shared cross-platform logic in common
.tsfiles (not suffixed with.nodeor.web)
Files:
src/kernels/deepnote/types.ts
🧬 Code graph analysis (1)
src/kernels/deepnote/deepnoteServerStarter.node.ts (2)
src/platform/common/types.ts (4)
IHttpClient(164-164)IHttpClient(165-171)IAsyncDisposableRegistry(232-232)IAsyncDisposableRegistry(233-235)src/platform/logging/index.ts (1)
logger(35-48)
| private async cleanupOrphanedProcesses(): Promise<void> { | ||
| try { | ||
| logger.info('Checking for orphaned deepnote-toolkit processes...'); | ||
| const processService = await this.processServiceFactory.create(undefined); | ||
|
|
||
| // Find all deepnote-toolkit server processes | ||
| let command: string; | ||
| let args: string[]; | ||
|
|
||
| if (process.platform === 'win32') { | ||
| // Windows: use tasklist and findstr | ||
| command = 'tasklist'; | ||
| args = ['/FI', 'IMAGENAME eq python.exe', '/FO', 'CSV', '/NH']; | ||
| } else { | ||
| // Unix-like: use ps and grep | ||
| command = 'ps'; | ||
| args = ['aux']; | ||
| } | ||
|
|
||
| const result = await processService.exec(command, args, { throwOnStdErr: false }); | ||
|
|
||
| if (result.stdout) { | ||
| const lines = result.stdout.split('\n'); | ||
| const pidsToKill: number[] = []; | ||
|
|
||
| for (const line of lines) { | ||
| // Look for processes running deepnote_toolkit server | ||
| if (line.includes('deepnote_toolkit') && line.includes('server')) { | ||
| // Extract PID based on platform | ||
| let pid: number | undefined; | ||
|
|
||
| if (process.platform === 'win32') { | ||
| // Windows CSV format: "python.exe","12345",... | ||
| const match = line.match(/"python\.exe","(\d+)"/); | ||
| if (match) { | ||
| pid = parseInt(match[1], 10); | ||
| } | ||
| } else { | ||
| // Unix format: user PID ... | ||
| const parts = line.trim().split(/\s+/); | ||
| if (parts.length > 1) { | ||
| pid = parseInt(parts[1], 10); | ||
| } | ||
| } | ||
|
|
||
| if (pid && !isNaN(pid)) { | ||
| pidsToKill.push(pid); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if (pidsToKill.length > 0) { | ||
| logger.info( | ||
| `Found ${pidsToKill.length} orphaned deepnote-toolkit processes: ${pidsToKill.join(', ')}` | ||
| ); | ||
| this.outputChannel.appendLine( | ||
| `Cleaning up ${pidsToKill.length} orphaned deepnote-toolkit process(es) from previous session...` | ||
| ); | ||
|
|
||
| // Kill each process | ||
| for (const pid of pidsToKill) { | ||
| try { | ||
| if (process.platform === 'win32') { | ||
| await processService.exec('taskkill', ['/F', '/T', '/PID', pid.toString()], { | ||
| throwOnStdErr: false | ||
| }); | ||
| } else { | ||
| await processService.exec('kill', ['-9', pid.toString()], { throwOnStdErr: false }); | ||
| } | ||
| logger.info(`Killed orphaned process ${pid}`); | ||
| } catch (ex) { | ||
| logger.warn(`Failed to kill process ${pid}: ${ex}`); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Don't kill active toolkit servers from other windows
Line 332 kicks off cleanupOrphanedProcesses() and the loop on Lines 356‑404 force-kills every deepnote_toolkit server PID it finds. Launching a second VS Code window will therefore terminate the live server owned by the first window, instantly dropping active notebooks. Please constrain this to genuine orphans (e.g., persist per-window ownership and skip PIDs still claimed, or verify that the parent process is gone) instead of blanket termination.
🤖 Prompt for AI Agents
In src/kernels/deepnote/deepnoteServerStarter.node.ts around lines 331-405, the
cleanup currently force-kills every process matching "deepnote_toolkit server",
which can terminate active servers from other VS Code windows; change the logic
to only kill genuine orphans by (1) persisting ownership when starting a server
(write a small per-window/session lock file in OS temp with server PID and a
unique session id) and (2) before killing a PID, verify ownership: if a lock
file exists for that PID and its session id differs from the current session,
skip it; additionally, verify the PID is actually orphaned by checking its
parent (on Unix use ps -o ppid= -p <pid> or /proc/<pid>/stat, on Windows query
ParentProcessId via wmic/PowerShell) and only kill if the parent is gone or
parent PID is 1 (or otherwise indicates orphaned), and log/skipped decisions;
implement cleanup of stale lock files when you successfully kill the process or
when the server you started exits.
Summary by CodeRabbit