fix(wsl): uninstall ends a recorded holder its command line proves, even without a start time (#36126)

This commit is contained in:
Neil
2026-10-29 15:27:09 -07:00
committed by GitHub
parent a3ddc75414
commit 82108b0da3
6 changed files with 121 additions and 16 deletions
@@ -44,9 +44,10 @@ function holder(release: () => Promise<WslDistroHolderAdoption[]>): WslDistroHol
describe('WSL holder release on uninstall', () => {
it('succeeds only when every recorded holder is proven gone', async () => {
vi.spyOn(console, 'warn').mockImplementation(() => {})
await expect(
releaseWslHoldersForUninstall(holder(async () => [{ distro: 'ubuntu', state: 'exited' }]))
).resolves.toBe(0)
const released = holder(async () => [{ distro: 'ubuntu', state: 'exited' }])
await expect(releaseWslHoldersForUninstall(released)).resolves.toBe(0)
// Uninstall trusts a matching holder command line; nothing later could release it.
expect(released.releaseAll).toHaveBeenCalledWith({ forUninstall: true })
await expect(releaseWslHoldersForUninstall(holder(async () => []))).resolves.toBe(0)
await expect(
releaseWslHoldersForUninstall(
@@ -11,7 +11,7 @@ const UNINSTALL_RELEASE_TIMEOUT_MS = 30_000
/** Exit code 0 only when every recorded holder is proven gone; unverifiable ones are left running. */
export async function releaseWslHoldersForUninstall(holder: WslDistroHolder): Promise<number> {
try {
const released = await holder.releaseAll()
const released = await holder.releaseAll({ forUninstall: true })
for (const { distro, state } of released) {
if (state !== 'exited') {
console.warn(`[wsl] uninstall left the ${distro} holder ${state}`)
@@ -3,10 +3,14 @@ import { isWslDistroHolderCommandLine } from './wsl-distro-holder-command'
import type { WslDistroHolderRecord } from './wsl-distro-holder-record'
import type { WslDistroHolderState } from './wsl-distro-holder-types'
/** Pid + creation time + command line must all agree; any one contradicting means pid reuse. */
/**
* Pid + creation time + command line must all agree; any one contradicting means pid reuse.
* `commandLineSuffices` (uninstall only): a matching command line is enough when a time is missing.
*/
export function classifyWslDistroHolderProcess(
record: WslDistroHolderRecord,
evidence: WindowsProcessEvidence
evidence: WindowsProcessEvidence,
{ commandLineSuffices = false } = {}
): WslDistroHolderState {
if (evidence.status === 'unavailable') {
return 'unverifiable'
@@ -25,5 +29,7 @@ export function classifyWslDistroHolderProcess(
return 'exited'
}
// A record written without a creation time never had one to compare; the command line decides.
return timeKnown || record.creationTimeMs === null ? 'live' : 'unverifiable'
return timeKnown || record.creationTimeMs === null || commandLineSuffices
? 'live'
: 'unverifiable'
}
+7 -1
View File
@@ -16,6 +16,12 @@ export type WslDistroHolderChild = EventEmitter & { readonly pid?: number; unref
export type WslDistroHolderAdoption = { distro: string; state: WslDistroHolderState }
/**
* Uninstall only: nothing will ever release a holder left now, so its marker and distro on the
* command line prove it ours without a start time, and an unreadable table is read once more.
*/
export type WslDistroHolderReleaseOptions = { forUninstall?: boolean }
export type WslDistroHolderDependencies = {
userDataPath: string
/** Test seam; production spawns `wsl.exe` detached through process-host. */
@@ -54,6 +60,6 @@ export type WslDistroHolder = {
/** Explicit stop only; the distro then idles out on WSL's own schedule. */
release(distro: string): Promise<WslDistroHolderState>
/** Every recorded holder (uninstall, hosts turned off); kills only verified holders. */
releaseAll(): Promise<WslDistroHolderAdoption[]>
releaseAll(options?: WslDistroHolderReleaseOptions): Promise<WslDistroHolderAdoption[]>
holderState(distro: string): Promise<WslDistroHolderState>
}
+74
View File
@@ -463,3 +463,77 @@ describe('createWslDistroHolder', () => {
expect(onHolderExit).toHaveBeenCalledWith('Alpine', { code: 1, signal: null })
})
})
describe('releaseAll for uninstall', () => {
beforeEach(() => {
vi.spyOn(console, 'warn').mockImplementation(() => {})
})
afterEach(() => {
vi.restoreAllMocks()
})
it('kills a matching holder with no start time on either side, and logs its pid', async () => {
host.processes.set(77, { commandLine: HOLDER_COMMAND('Alpine'), startedAtMs: 5_000 })
await writeRecord(record({ creationTimeMs: null }))
host.processes.set(88, { commandLine: HOLDER_COMMAND('Debian'), startedAtMs: null })
await writeRecord(record({ distro: 'Debian', pid: 88, creationTimeMs: 6_000 }))
const released = await makeHolder().releaseAll({ forUninstall: true })
expect(released.sort((a, b) => a.distro.localeCompare(b.distro))).toEqual([
{ distro: 'alpine', state: 'exited' },
{ distro: 'debian', state: 'exited' }
])
expect(host.killed.sort()).toEqual([77, 88])
expect(await recordFiles()).toEqual([])
expect(console.warn).toHaveBeenCalledWith(
'[wsl] uninstall: Alpine holder pid 77 was live, now exited'
)
})
it('never kills a pid whose command line is not the holder for the recorded distro', async () => {
host.processes.set(77, { commandLine: 'C:\\Windows\\notepad.exe', startedAtMs: 5_000 })
await writeRecord(record({ creationTimeMs: null }))
host.processes.set(88, { commandLine: HOLDER_COMMAND('Ubuntu'), startedAtMs: null })
await writeRecord(record({ distro: 'Debian', pid: 88, creationTimeMs: null }))
const released = await makeHolder().releaseAll({ forUninstall: true })
expect(released.sort((a, b) => a.distro.localeCompare(b.distro))).toEqual([
{ distro: 'alpine', state: 'exited' },
{ distro: 'debian', state: 'exited' }
])
expect(host.killed).toEqual([])
})
it('still trusts a known start time that contradicts the record', async () => {
host.processes.set(77, { commandLine: HOLDER_COMMAND('Alpine'), startedAtMs: 9_999 })
await writeRecord(record())
expect(await makeHolder().releaseAll({ forUninstall: true })).toEqual([
{ distro: 'alpine', state: 'exited' }
])
expect(host.killed).toEqual([])
})
it('reads an unavailable process table once more before giving up', async () => {
host.processes.set(77, { commandLine: HOLDER_COMMAND('Alpine'), startedAtMs: 5_000 })
await writeRecord(record())
host.tableFails = true
const sleep = vi.fn(async () => {
host.tableFails = false
})
expect(await makeHolder({ sleep }).releaseAll({ forUninstall: true })).toEqual([
{ distro: 'alpine', state: 'exited' }
])
expect(host.killed).toEqual([77])
host.processes.set(78, { commandLine: HOLDER_COMMAND('Alpine'), startedAtMs: 5_000 })
await writeRecord(record({ pid: 78 }))
host.tableFails = true
expect(await makeHolder().releaseAll({ forUninstall: true })).toEqual([
{ distro: 'alpine', state: 'unverifiable' }
])
expect(host.killed).toEqual([77])
expect(await recordFiles()).toEqual(['alpine.json'])
expect(console.warn).toHaveBeenCalledWith(
'[wsl] uninstall: Alpine holder pid 78 was unverifiable, now unverifiable'
)
})
})
+26 -8
View File
@@ -31,6 +31,7 @@ import type {
WslDistroHolderChild,
WslDistroHolderDependencies,
WslDistroHolderEnsureResult,
WslDistroHolderReleaseOptions,
WslDistroHolderState
} from './wsl-distro-holder-types'
@@ -187,11 +188,20 @@ export function createWslDistroHolder(dependencies: WslDistroHolderDependencies)
async function stopRecordedHolder(
record: WslDistroHolderRecord,
{ spawnedHere = false } = {}
{ spawnedHere = false, forUninstall = false } = {}
): Promise<WslDistroHolderState> {
let state = classifyWslDistroHolderProcess(record, await queryProcess(record.pid))
const classify = async (): Promise<WslDistroHolderState> =>
classifyWslDistroHolderProcess(record, await queryProcess(record.pid), {
commandLineSuffices: forUninstall
})
let state = await classify()
if (state === 'unverifiable' && forUninstall) {
await sleep(BOOT_POLL_INTERVAL_MS)
state = await classify()
}
// A command line alone can't tell our holder from a reused pid running the same argv.
const provenOurs = spawnedHere || record.creationTimeMs !== null
const provenOurs = spawnedHere || record.creationTimeMs !== null || forUninstall
const stateBeforeKill = state
if (state === 'live' && provenOurs) {
try {
// WSL 2.7.10 relaunches a same-argv child; killing the recorded parent ends both.
@@ -201,7 +211,7 @@ export function createWslDistroHolder(dependencies: WslDistroHolderDependencies)
}
const deadline = monotonicNow() + RELEASE_EXIT_TIMEOUT_MS
for (;;) {
state = classifyWslDistroHolderProcess(record, await queryProcess(record.pid))
state = await classify()
// Mid-exit rows may retain PID but lose command line; only exited proves termination.
if (state === 'exited' || monotonicNow() >= deadline) {
break
@@ -212,6 +222,11 @@ export function createWslDistroHolder(dependencies: WslDistroHolderDependencies)
if (state === 'exited') {
await removeWslDistroHolderRecord(userDataPath, record.distro)
}
if (forUninstall) {
console.warn(
`[wsl] uninstall: ${record.distro} holder pid ${record.pid} was ${stateBeforeKill}, now ${state}`
)
}
return state === 'live' && !provenOurs ? 'unverifiable' : state
}
@@ -289,10 +304,10 @@ export function createWslDistroHolder(dependencies: WslDistroHolderDependencies)
release: (distro) => releaseRecorded(distro),
async releaseAll() {
async releaseAll(options) {
const distros = await listWslDistroHolderRecordDistros(userDataPath)
return Promise.all(
distros.map(async (distro) => ({ distro, state: await releaseRecorded(distro) }))
distros.map(async (distro) => ({ distro, state: await releaseRecorded(distro, options) }))
)
},
@@ -302,7 +317,10 @@ export function createWslDistroHolder(dependencies: WslDistroHolderDependencies)
}
return holder
function releaseRecorded(distro: string): Promise<WslDistroHolderState> {
function releaseRecorded(
distro: string,
options: WslDistroHolderReleaseOptions = {}
): Promise<WslDistroHolderState> {
return serialize(distro, async () => {
const read = await readWslDistroHolderRecord(userDataPath, distro)
if (read.status === 'unreadable') {
@@ -312,7 +330,7 @@ export function createWslDistroHolder(dependencies: WslDistroHolderDependencies)
await removeWslDistroHolderRecord(userDataPath, distro)
return 'exited'
}
return stopRecordedHolder(read.record)
return stopRecordedHolder(read.record, options)
})
}
}