From 8fe31c930eb3422c439fe4168b2f42d30c8a9938 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Tue, 27 Oct 2026 14:42:00 -0700 Subject: [PATCH] feat(wsl): note why an idle distro stays held, and fail the holder's scan closed (#34534) * feat(wsl): note why an idle distro stays held, and fail the holder's scan closed When user work keeps the holder from releasing, it writes the first few process names (at most 5) to ~/.orca/wsl-holder-blocked, replaced whole and cleared on release, for the desktop to read later. A /proc without the holder's own entry, or a status file it cannot parse, now keeps the hold instead of reading as idle. * fix(wsl): count an unreadable /proc entry only when it is the user's own Review of #34534: under hidepid=noaccess root's status files are unreadable, so each read as this user's work and the distro never released. An unreadable entry now counts only when its directory belongs to this user. The holder also clears a half-written note on start and rewrites the note only when it changed. --- src/main/wsl/wsl-distro-holder-command.ts | 46 ++++++--- src/main/wsl/wsl-distro-holder-script.test.ts | 97 +++++++++++++++++-- src/shared/orcad-idle-exit.ts | 3 + 3 files changed, 126 insertions(+), 20 deletions(-) diff --git a/src/main/wsl/wsl-distro-holder-command.ts b/src/main/wsl/wsl-distro-holder-command.ts index a3a603ac09a..6ef57c2733e 100644 --- a/src/main/wsl/wsl-distro-holder-command.ts +++ b/src/main/wsl/wsl-distro-holder-command.ts @@ -1,4 +1,7 @@ -import { ORCAD_WSL_HOLDER_RELEASE_FILENAME } from '../../shared/orcad-idle-exit' +import { + ORCAD_WSL_HOLDER_BLOCKED_FILENAME, + ORCAD_WSL_HOLDER_RELEASE_FILENAME +} from '../../shared/orcad-idle-exit' import { buildWslExecArgs } from '../../shared/wsl-login-shell-command' /** `$0` of the holder's shell; the token that identifies our holder in a process command line. */ @@ -14,6 +17,9 @@ export const WSL_DISTRO_HOLDER_POLL_SECONDS = 15 */ export const WSL_DISTRO_HOLDER_IGNORED_PROCESS_NAMES = ['systemd', '(sd-pam)', 'dbus-daemon'] +/** At most this many process names go into the blocked note, so a busy distro stays cheap. */ +export const WSL_DISTRO_HOLDER_BLOCKED_NAMES_MAX = 5 + // Why short sleeps: busybox `sleep` (Alpine) rejects `infinity`. Why `rm` without -f: it claims the // request atomically, so an orcad that starts and clears it first keeps the holder (and the distro). // `~/.orca` is the WSL host's data root (orcad-remote-context); without HOME the path never exists. @@ -23,30 +29,44 @@ export function buildWslDistroHolderScript( ): string { const ignored = WSL_DISTRO_HOLDER_IGNORED_PROCESS_NAMES.map((name) => `'${name}'`).join('|') return [ - `f="$HOME/.orca/${ORCAD_WSL_HOLDER_RELEASE_FILENAME}"; me=$(id -u)`, + `f="$HOME/.orca/${ORCAD_WSL_HOLDER_RELEASE_FILENAME}"; b="$HOME/.orca/${ORCAD_WSL_HOLDER_BLOCKED_FILENAME}"; me=$(id -u)`, // A new holder means a client wants the distro: an older orcad's request no longer stands. - 'rm -f "$f"', + // Also any note half-written by a holder killed mid-write. + 'rm -f "$f" "$b" "$b".*', // Fails toward holding: any live process of this user outside the holder itself counts, so a - // distro whose default user is root never releases. One exception: with /proc mounted - // hidepid, another user's container shim is invisible and does not hold. + // distro whose default user is root never releases; so does a /proc it cannot read, or a + // status file it cannot parse. One exception: with /proc mounted hidepid, another user's + // container shim is invisible and does not hold. Leaves the first names it found in $why. 'user_busy() {', + 'why=; c=0', + `[ -r "${procRoot}/$$/status" ] || { why=proc-unreadable; return 0; }`, `for d in ${procRoot}/[0-9]*; do`, + '[ "${d##*/}" = "$$" ] && continue', 'n= s= pp= u=', - 'while read -r k v r; do case $k in Name:) n=$v ;; State:) s=$v ;; PPid:) pp=$v ;; Uid:) u=$v; break ;; esac', - 'done 2>/dev/null < "$d/status" || continue', + '{ while read -r k v r; do case $k in Name:) n=$v ;; State:) s=$v ;; PPid:) pp=$v ;; Uid:) u=$v; break ;; esac', + 'done < "$d/status"; } 2>/dev/null', + // A process that vanished mid-scan is gone; one still there that cannot be read counts only + // when its entry is this user's (hidepid=noaccess hides root's status files, not their dirs). + '[ -n "$u" ] || { [ -d "$d" ] || continue; set -- $(ls -nd "$d" 2>/dev/null); [ "${3:-}" = "$me" ] || continue; n=${n:-unreadable}; u=$me; }', // A running container is work whoever owns it (docker's shims run as root). - 'case $n in containerd-shim*) return 0 ;; esac', - '[ "$u" = "$me" ] && [ "${d##*/}" != "$$" ] && [ "$pp" != "$$" ] || continue', + 'case $n in containerd-shim*) ;; *)', + '[ "$u" = "$me" ] && [ "$pp" != "$$" ] || continue', 'case $s in Z*) continue ;; esac', - `case $n in ${ignored}) continue ;; esac`, - 'return 0', - 'done; return 1; }', + `case $n in ${ignored}) continue ;; esac ;;`, + 'esac', + `why="$why $n"; c=$((c + 1)); [ "$c" -lt ${WSL_DISTRO_HOLDER_BLOCKED_NAMES_MAX} ] || return 0`, + 'done; [ -n "$why" ]; }', `while :; do sleep ${pollSeconds}`, '[ -f "$f" ] || continue', 'p=; read -r p < "$f"', "case $p in ''|*[!0-9]*) continue ;; esac", 'while kill -0 "$p" 2>/dev/null; do sleep 1; done', - 'user_busy && continue', + // Why still held, for the desktop to read later (replaced whole); cleared on release. + 'if user_busy; then', + // Rewritten only when it changed, so a long hold costs no writes. + '[ "$(cat "$b" 2>/dev/null)" = "${why# }" ] || { printf \'%s\\n\' "${why# }" > "$b.$$" && mv -f "$b.$$" "$b"; } 2>/dev/null', + 'continue; fi', + 'rm -f "$b"', 'rm "$f" 2>/dev/null && exit 0', 'done' ].join('\n') diff --git a/src/main/wsl/wsl-distro-holder-script.test.ts b/src/main/wsl/wsl-distro-holder-script.test.ts index 89a76e834a9..c6b18f05084 100644 --- a/src/main/wsl/wsl-distro-holder-script.test.ts +++ b/src/main/wsl/wsl-distro-holder-script.test.ts @@ -1,11 +1,23 @@ import { spawn, type ChildProcess } from 'node:child_process' -import { existsSync, mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { + chmodSync, + existsSync, + mkdirSync, + mkdtempSync, + readFileSync, + rmSync, + writeFileSync +} from 'node:fs' import { tmpdir, userInfo } from 'node:os' import { join } from 'node:path' import { afterEach, beforeEach, describe, expect, it } from 'vitest' -import { ORCAD_WSL_HOLDER_RELEASE_FILENAME } from '../../shared/orcad-idle-exit' +import { + ORCAD_WSL_HOLDER_BLOCKED_FILENAME, + ORCAD_WSL_HOLDER_RELEASE_FILENAME +} from '../../shared/orcad-idle-exit' import { buildWslDistroHolderScript, + WSL_DISTRO_HOLDER_BLOCKED_NAMES_MAX, WSL_DISTRO_HOLDER_IGNORED_PROCESS_NAMES, WSL_DISTRO_HOLDER_MARKER } from './wsl-distro-holder-command' @@ -37,13 +49,20 @@ function track(child: ChildProcess): ChildProcess { return child } -function startHolder(env: NodeJS.ProcessEnv = { HOME: home, PATH: process.env.PATH }) { +function startHolder( + env: NodeJS.ProcessEnv = { HOME: home, PATH: process.env.PATH }, + options: { procReadable?: boolean } = {} +) { const child = track( spawn('/bin/sh', ['-c', buildWslDistroHolderScript(1, proc), WSL_DISTRO_HOLDER_MARKER], { env, stdio: 'ignore' }) ) + // A real /proc always shows the holder itself; without that entry the scan fails closed. + if (options.procReadable !== false && child.pid !== undefined) { + fakeProcess(child.pid, { name: 'sh', ppid: 1 }) + } const exited = new Promise((resolve) => child.once('exit', resolve)) return { child, exited } } @@ -64,7 +83,7 @@ function fakeProcess( fields: { name?: string; state?: string; ppid?: number; uid?: number } = {} ): string { const dir = join(proc, String(pid)) - mkdirSync(dir) + mkdirSync(dir, { recursive: true }) writeFileSync( join(dir, 'status'), `Name:\t${fields.name ?? 'tmux: server'}\nState:\t${fields.state ?? 'S (sleeping)'}\n` + @@ -73,9 +92,11 @@ function fakeProcess( return dir } +const blockedPath = (): string => join(home, '.orca', ORCAD_WSL_HOLDER_BLOCKED_FILENAME) + /** A holder that already ran its start-up `rm -f`, then a request from an orcad that has exited. */ -async function holderWithRequest() { - const holder = startHolder() +async function holderWithRequest(options: { procReadable?: boolean; path?: string } = {}) { + const holder = startHolder({ HOME: home, PATH: options.path ?? process.env.PATH }, options) expect(await within(300, holder.exited)).toBe('running') const orcad = track(spawn('/bin/sh', ['-c', 'exit 0'], { stdio: 'ignore' })) await new Promise((resolve) => orcad.once('exit', resolve)) @@ -161,11 +182,73 @@ describe.skipIf(process.platform === 'win32')('WSL distro holder script', () => fakeProcess(pid++, { uid: me + 1 }) const holder = startHolder() fakeProcess(pid++, { name: 'sleep', ppid: holder.child.pid }) - fakeProcess(holder.child.pid ?? pid++, { name: 'sh' }) expect(await within(300, holder.exited)).toBe('running') const orcad = track(spawn('/bin/sh', ['-c', 'exit 0'], { stdio: 'ignore' })) await new Promise((resolve) => orcad.once('exit', resolve)) writeFileSync(releasePath(), `${orcad.pid}\n`) expect(await within(3_000, holder.exited)).toBe(0) }) + + it('notes what keeps it holding, and clears the note once it releases', async () => { + const job = fakeProcess(4242, { name: 'node' }) + const { exited } = await holderWithRequest() + await expect.poll(() => existsSync(blockedPath()), { timeout: 5_000 }).toBe(true) + expect(readFileSync(blockedPath(), 'utf8')).toBe('node\n') + rmSync(job, { recursive: true }) + expect(await within(3_000, exited)).toBe(0) + expect(existsSync(blockedPath())).toBe(false) + }) + + it('keeps the note bounded however much the user runs', async () => { + for (let pid = 6000; pid < 6000 + 3 * WSL_DISTRO_HOLDER_BLOCKED_NAMES_MAX; pid++) { + fakeProcess(pid, { name: `job${pid}` }) + } + const { exited } = await holderWithRequest() + await expect.poll(() => existsSync(blockedPath()), { timeout: 5_000 }).toBe(true) + expect(readFileSync(blockedPath(), 'utf8').trim().split(' ')).toHaveLength( + WSL_DISTRO_HOLDER_BLOCKED_NAMES_MAX + ) + expect(await within(1_500, exited)).toBe('running') + }) + + it('fails closed when /proc cannot be read', async () => { + const { exited } = await holderWithRequest({ procReadable: false }) + expect(await within(2_500, exited)).toBe('running') + expect(readFileSync(blockedPath(), 'utf8')).toBe('proc-unreadable\n') + }) + + it('fails closed on a status file it cannot parse', async () => { + const dir = join(proc, '7000') + mkdirSync(dir) + writeFileSync(join(dir, 'status'), 'Name:\tmystery\n') + const { exited } = await holderWithRequest() + expect(await within(2_500, exited)).toBe('running') + expect(readFileSync(blockedPath(), 'utf8')).toBe('mystery\n') + }) + + it("counts an unreadable entry only when it is the user's own (hidepid=noaccess)", async () => { + // Root's status files are unreadable under hidepid; their directories still show their owner. + const dir = join(proc, '7100') + mkdirSync(dir) + writeFileSync(join(dir, 'status'), 'Name:\tsecret\n') + chmodSync(join(dir, 'status'), 0o000) + // The user's own unreadable entry holds. + const own = await holderWithRequest() + expect(await within(2_500, own.exited)).toBe('running') + own.child.kill('SIGKILL') + // Seen by another user, the same entry is someone else's and does not hold. + const bin = join(home, 'bin') + mkdirSync(bin) + writeFileSync(join(bin, 'id'), `#!/bin/sh\necho ${me + 1}\n`, { mode: 0o755 }) + const other = await holderWithRequest({ path: `${bin}:${process.env.PATH}` }) + expect(await within(3_000, other.exited)).toBe(0) + }) + + it('cleans up a note a killed holder left half-written', async () => { + const stale = `${blockedPath()}.12345` + writeFileSync(stale, 'half') + const { exited } = startHolder() + expect(await within(500, exited)).toBe('running') + expect(existsSync(stale)).toBe(false) + }) }) diff --git a/src/shared/orcad-idle-exit.ts b/src/shared/orcad-idle-exit.ts index 51ec881b3c4..2d8fc972bdb 100644 --- a/src/shared/orcad-idle-exit.ts +++ b/src/shared/orcad-idle-exit.ts @@ -34,3 +34,6 @@ export type OrcadIdleStopRecord = z.infer * terminal daemon. The distro holder ends once that pid exits, so WSL may stop the idle distro. */ export const ORCAD_WSL_HOLDER_RELEASE_FILENAME = 'wsl-holder-release' + +/** Beside the release request: the first few things keeping an idle distro held, one line. */ +export const ORCAD_WSL_HOLDER_BLOCKED_FILENAME = 'wsl-holder-blocked'