Fix Jupyter process leak and start/resume desync dead-end in module workflow - #2092
Closed
Shashank-Tripathi-07 wants to merge 6 commits into
Closed
Conversation
…Lab processes Bug Every tito module start (without --no-jupyter), resume, or view call launches a brand new jupyter lab subprocess.Popen with no PID tracking, no cleanup, and no check for whether one is already running. Over a normal working session where a student opens several modules, these accumulate indefinitely: confirmed 45 leaked processes after routine module-by-module testing, and separately confirmed that a build-up of ~38 live kernels was enough to make the Jupyter Lab UI itself sluggish (menu clicks timing out). Fix Track the launched Jupyter Lab process's PID in .tito/jupyter.pid. Before launching a new one, check whether that PID is still alive and is actually still a jupyter process (guards against a recycled PID being mistaken for a live server). If so, reuse it: tell the student which notebook to open in their existing tab instead of spawning another server. Only launch a new process if no tracked server is currently running. Testing Verified against a fresh dev install with all 20 modules completed: - First tito module view 01: launches Jupyter Lab as before, records its PID. - Second tito module view 01 (simulating a student opening another module in the same session): before the fix, this would spawn a second full Jupyter Lab server. After the fix, detects the existing PID, prints "Jupyter Lab is already running (pid N)" with the notebook to open, and spawns nothing. Confirmed via process listing that the jupyter.exe count stayed at 1 across both calls.
The psutil-unavailable fallback in _running_jupyter_pid used os.kill(pid, 0), assuming POSIX signal-0 "probe only" semantics. On Windows, os.kill does not implement that: it calls TerminateProcess for any signal value, including 0. The fallback path meant to check whether a Jupyter server was still running would instead kill it. psutil is already a required project dependency (requirements.txt, tito setup's own package list), so this path should only be reached in a broken environment. Removed the unsafe probe: if psutil can't be imported, treat it the same as "can't tell" and return None, so the caller launches a fresh server instead. Worst case is a duplicate server (the same behavior this PR is fixing in the common case), never silently killing a running one.
… only The previous commit's fix was correct (removed the os.kill(pid, 0) Windows footgun), but its comment claimed psutil is "already a required project dependency" -- it isn't, in either pyproject.toml or requirements.txt (only tito setup's own separate package-install list includes it, which is not the same thing). Since psutil is absent by default, _running_jupyter_pid() always hit the "can't verify" path and always returned None, meaning the leak-prevention logic silently no-opped and the original leak this PR set out to fix was still present in the common case -- just no longer dangerous. Replaced the psutil dependency entirely with a stdlib-only check: - Windows: query the process via the Win32 API (OpenProcess + QueryFullProcessImageNameW through ctypes) to confirm both liveness and that it's actually a Jupyter process, without ever sending it any kind of signal. - POSIX: os.kill(pid, 0) is a real, safe liveness probe there (unlike Windows). Best-effort cmdline verification via /proc/<pid>/cmdline on Linux; falls back to "alive is good enough" on platforms without /proc (macOS/BSD), since there's no portable stdlib way to get a process's command line there. Testing Verified on this Windows machine, launching and reusing detection without any timeout-wrapper interference (an earlier test attempt using a `timeout N` wrapper around the launching command turned out to kill the detached Jupyter child once the wrapped command completed, a Windows job-object artifact of that specific test harness, not a bug in this code -- confirmed by retesting with a plain backgrounded call instead): - First tito module view 01: launches Jupyter Lab, PID confirmed alive via Get-Process independently afterward. - Second tito module view 01: correctly detects the existing PID via the new Win32-API-based check and prints "Jupyter Lab is already running (pid N)" instead of launching a second server. jupyter.exe process count for this project confirmed to stay at 1 across both calls. - Confirmed psutil is not installed as a dependency anywhere in this path (grepped pyproject.toml and requirements.txt).
… check The Windows liveness check called kernel32.OpenProcess/QueryFullProcessImageNameW/CloseHandle through ctypes without declaring their argtypes/restype. HANDLE is pointer-width (64-bit on Win64); without an explicit restype, ctypes defaults OpenProcess's return to a 32-bit signed int and truncates it. That happens to be harmless for the small handle values Windows hands out in practice, but it is not correct per the Win32 API contract. Declared the real argtypes/restype for OpenProcess, QueryFullProcessImageNameW, and CloseHandle so the kernel-level values (HANDLE, DWORD, BOOL) are marshalled correctly instead of relying on that truncation happening to be safe.
… desync started_modules/completed_modules tracking in .tito/progress.json and the actual notebook under modules/ can go out of sync (e.g. tito system reset --keep-progress clears modules/ but intentionally preserves tracking). When that happens, module start refused with 'already started', pointing to module resume, and module resume then failed inside _open_jupyter with 'directory not found', pointing back at nothing. Neither message ever mentioned the actual fix (module reset --force). Both commands now check whether the notebook actually exists before trusting the tracking flag, and recreate it from src/ when it doesn't, the same way a genuinely new module is created. The normal already-started path (notebook genuinely present) is unchanged.
This was referenced Aug 19, 2026
github-actions
Bot
added
area: tinytorch
profvjreddi
added a commit
that referenced
this pull request
Aug 31, 2026
Land four more contributor PRs, each audited by running it rather than reading it, with the defects found repaired here: #2015 bare 'tito package reset' now actually resets #2023 conftest validates all 20 module exports #2080 link-check counts from the failure sections only #2092 Jupyter server reuse + start/resume desync recovery Three of the four had a real defect that only surfaced under execution: #2015 --force before the SUBCOMMAND token was silently dropped by the subparser default, then blocked on input() (EOFError in CI) #2023 the module-20 registry path never resolved, so every pytest run on a fully-exported tree printed a false 'not exported' warning, and two of the PR's own tests asserted the wrong value #2092 the pid check returned True for any live process on macOS, so a recycled pid would permanently block launching Jupyter #2080 needed no repair: its three claims check out against real lychee 0.23.0 output.
Contributor
|
Thanks @Shashank-Tripathi-07. Integrated into dev, CI green. The Windows note in your PR was right, and it turns out the POSIX side needed the same care. Since the pid file outlives the server it names and pids get recycled, that meant Fixed by falling back to |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Combines 2 previously-separate PRs (#2011, #2026), both of which touch
tito/commands/module/workflow.py. Combining them avoids two PRs churning the same file back to back.Changes
fix(tinytorch): tito module start/resume/view leak untracked Jupyter Lab processes #2011 (4 commits, including 3 follow-up fixes to the liveness check itself) — Every
tito module start(without--no-jupyter),resume, orviewlaunched a brand newjupyter labsubprocess with no tracking, no cleanup, and no check for whether one was already running. Confirmed 45 leaked processes after routine module-by-module testing. Fixed by tracking the launched process's PID in.tito/jupyter.pidand reusing an already-running server instead of spawning a duplicate. The liveness check itself went through a few iterations during review/testing:os.kill(pid, 0)isn't reliable on Windows, so it moved to a stdlib-only cross-platform check, plus a follow-up fix for how the kernel-level PID value was being handled.fix(tinytorch): stop module start/resume dead-ending on tracking/disk desync #2026 —
tito module startandtito module resumecould deadlock each other when.tito/progress.json's tracking (started_modules) desynced from the actual notebook on disk undermodules/. Each command's failure message pointed at the other, and neither ever mentioned the actual fix.start_moduleandresume_modulenow check whether the notebook directory actually exists before trusting the tracking flag, and recreate it fromsrc/when it doesn't, instead of dead-ending.Testing
Both fixes were manually verified against a fresh
devinstall (process-count checks via process listing for #2011, notebook-recreation checks for #2026) at the time each was originally written. Recombined here with no logic changes;py_compileclean, no merge conflicts across the two originals (fully sequential, same-file changes).