diff --git a/scripts/ci/dev-up.test.mjs b/scripts/ci/dev-up.test.mjs index 0d304f69b..47adfd494 100644 --- a/scripts/ci/dev-up.test.mjs +++ b/scripts/ci/dev-up.test.mjs @@ -1806,6 +1806,188 @@ for (const platform of platforms) { } }) + // #1898/#2378: port release is bounded by an elapsed-time deadline, not a fixed iteration count. + // Both cases stop a synthetic state whose recorded PID is already gone, so the only thing under + // test is the port-release stage. + async function writeReapedState(fixture, { apiPort, frontendPort }) { + const runId = '22222222-2222-4222-8222-222222222222' + const deadProcess = spawn(process.execPath, ['-e', 'process.exit(0)'], { + stdio: 'ignore', + windowsHide: true, + }) + const deadPid = deadProcess.pid + await new Promise((resolve) => deadProcess.once('exit', resolve)) + await mkdir(fixture.stateDir, { recursive: true }) + const prefix = join(fixture.stateDir, `dev-up-${runId}`) + await writeFile( + fixture.stateFile, + `${JSON.stringify( + { + schemaVersion: 1, + runId, + apiPort, + frontend: { url: `http://localhost:${frontendPort}/`, port: frontendPort }, + logs: { + apiStdout: `${prefix}-api.stdout.log`, + apiStderr: `${prefix}-api.stderr.log`, + frontendStdout: `${prefix}-frontend.stdout.log`, + frontendStderr: `${prefix}-frontend.stderr.log`, + }, + processes: [ + { role: 'api', pid: deadPid, name: 'node', creationToken: 'reaped-before-this-stop' }, + { role: 'frontend', pid: deadPid, name: 'node', creationToken: 'reaped-before-this-stop' }, + ], + }, + null, + 2, + )}\n`, + ) + } + + test( + `${platform.name}: a frontend port released after the old fixed budget still stops cleanly`, + { concurrency: false }, + async () => { + const fixture = await createFixture(platform) + const apiPort = await getFreePort() + let frontendPort + do frontendPort = await getFreePort() + while (frontendPort === apiPort) + // Held out-of-process because runLauncher blocks this event loop: the holder releases the + // port 12s after the launcher starts, well past the 5s fixed budget the old Wait-PortRelease + // allowed (50 x 100ms), so this case fails on the pre-fix launchers and passes on these. + const holder = spawn( + process.execPath, + [ + '-e', + "const net=require('node:net');const s=net.createServer();s.listen(Number(process.argv[1]),'127.0.0.1',()=>{console.log('ready');setTimeout(()=>process.exit(0),12000)})", + String(frontendPort), + ], + { stdio: ['ignore', 'pipe', 'ignore'], windowsHide: true }, + ) + try { + await new Promise((resolve, reject) => { + holder.stdout.once('data', resolve) + holder.once('exit', () => reject(new Error('port holder exited before it was ready'))) + }) + assert.equal(await canBind(frontendPort, '127.0.0.1'), false, 'port holder did not occupy the port') + await writeReapedState(fixture, { apiPort, frontendPort }) + const result = runLauncher(platform, fixture, { + stop: true, + timeout: 40_000, + env: { TASKDECK_DEV_UP_PORT_RELEASE_TIMEOUT_MS: '20000' }, + }) + assert.ifError(result.error) + assert.equal(result.status, 0, combinedOutput(result)) + assert.match(combinedOutput(result), /Stack stopped/) + assert.equal(await readOptional(fixture.stateFile), null) + } finally { + holder.kill('SIGKILL') + if (existsSync(fixture.stateFile)) { + runLauncher(platform, fixture, { + stop: true, + env: { TASKDECK_DEV_UP_PORT_RELEASE_TIMEOUT_MS: '1500' }, + }) + } + await removeFixture(fixture) + } + }, + ) + + test( + `${platform.name}: a live listener still holding the frontend port fails closed`, + { concurrency: false }, + async () => { + const fixture = await createFixture(platform) + const apiPort = await getFreePort() + const foreign = await listenForeign('127.0.0.1') + const frontendPort = foreign.address().port + try { + await writeReapedState(fixture, { apiPort, frontendPort }) + const result = runLauncher(platform, fixture, { + stop: true, + timeout: 30_000, + env: { TASKDECK_DEV_UP_PORT_RELEASE_TIMEOUT_MS: '1500' }, + }) + assertFailedClosed(result) + assert.match(combinedOutput(result), /Frontend port .* is still occupied/) + // Either fail-closed reason is acceptable: a readable listener inventory names the live owner, + // and a platform without one (Git Bash on Windows has neither lsof nor ss) must still refuse. + assert.match(combinedOutput(result), /held by a live|could not be inventoried/) + assert.equal(existsSync(fixture.stateFile), true, 'PID state was dropped despite a live listener') + assert.equal(foreign.listening, true, 'the live listener was disturbed') + } finally { + // No cleanup stop here: the listener is still up on purpose, so another stop would only + // burn the deadline again. removeFixture discards the whole state directory. + await new Promise((resolve) => foreign.close(resolve)) + await removeFixture(fixture) + } + }, + ) + + // An unprivileged inventory can see that a socket is listening without being able to name its + // owner: `ss -p` omits `users:(...)` for another account's socket, and `lsof` cannot see it at + // all. An unattributable owner must never be read as "nothing is listening" - otherwise a + // root-owned listener (docker-proxy, a systemd unit on 5000/5173) would be reported as a clean + // stop and the PID file removed. Stubs reproduce that exact shape. + if (platform.name === 'Bash') { + test( + `${platform.name}: a listening socket with no attributable owner still fails closed`, + { concurrency: false }, + async () => { + const fixture = await createFixture(platform) + const apiPort = await getFreePort() + const foreign = await listenForeign('127.0.0.1') + const frontendPort = foreign.address().port + try { + // Prints a LISTEN row for the frontend port only - and never a `pid=` field. + await writeFile( + join(fixture.fakeBin, 'ss'), + [ + '#!/usr/bin/env bash', + 'for arg in "$@"; do', + ' case "$arg" in', + ' *":$TASKDECK_TEST_UNATTRIBUTABLE_PORT")', + ' printf \'LISTEN 0 511 0.0.0.0:%s 0.0.0.0:*\\n\' "$TASKDECK_TEST_UNATTRIBUTABLE_PORT"', + ' exit 0 ;;', + ' esac', + 'done', + 'exit 0', + '', + ].join('\n'), + ) + await chmod(join(fixture.fakeBin, 'ss'), 0o755) + // Deny the lsof attribution fallback too, so the unattributable path is deterministic + // whether or not the host has a real lsof. + await writeFile(join(fixture.fakeBin, 'lsof'), '#!/usr/bin/env bash\nexit 1\n') + await chmod(join(fixture.fakeBin, 'lsof'), 0o755) + + await writeReapedState(fixture, { apiPort, frontendPort }) + const result = runLauncher(platform, fixture, { + stop: true, + timeout: 30_000, + env: { + TASKDECK_DEV_UP_PORT_RELEASE_TIMEOUT_MS: '1500', + TASKDECK_TEST_UNATTRIBUTABLE_PORT: String(frontendPort), + }, + }) + assertFailedClosed(result) + assert.match(combinedOutput(result), /still held by a live listener/) + assert.match(combinedOutput(result), /Frontend port .* is still occupied/) + assert.equal( + existsSync(fixture.stateFile), + true, + 'PID state was dropped for a listener that could not be attributed to a PID', + ) + assert.equal(foreign.listening, true, 'the unattributable listener was disturbed') + } finally { + await new Promise((resolve) => foreign.close(resolve)) + await removeFixture(fixture) + } + }, + ) + } + test(`${platform.name}: high-volume stdout and stderr cannot deadlock marker acceptance`, { concurrency: false, timeout: 60_000 }, async () => { const fixture = await createFixture(platform) const apiPort = await getFreePort() diff --git a/scripts/dev-up.ps1 b/scripts/dev-up.ps1 index 182284fef..b0b5b2973 100644 --- a/scripts/dev-up.ps1 +++ b/scripts/dev-up.ps1 @@ -58,6 +58,8 @@ $FrontendDir = Join-Path $RepoRoot "frontend/taskdeck-web" $DataDir = Join-Path $env:LOCALAPPDATA "Taskdeck" $DevDbPath = Join-Path $DataDir "taskdeck-dev.db" $PidFile = Join-Path $DataDir "dev-up.pids" +# Hosted-safe default; override with TASKDECK_DEV_UP_PORT_RELEASE_TIMEOUT_MS. +$DefaultPortReleaseTimeoutMs = 30000 $OperationLockFile = Join-Path $DataDir "dev-up.operation.lock" $MinimumNodeVersion = [version]"24.13.1" @@ -364,13 +366,110 @@ function Test-PortBindable { return $true } +function Get-PortReleaseTimeoutMs { + $raw = $env:TASKDECK_DEV_UP_PORT_RELEASE_TIMEOUT_MS + if (-not [string]::IsNullOrWhiteSpace($raw)) { + $parsed = 0 + if ([int]::TryParse($raw.Trim(), [ref]$parsed) -and $parsed -ge 0) { return $parsed } + Write-DevWarning "Ignoring invalid TASKDECK_DEV_UP_PORT_RELEASE_TIMEOUT_MS '$raw'; using $DefaultPortReleaseTimeoutMs ms." + } + return $DefaultPortReleaseTimeoutMs +} + +# Inventory of TCP LISTEN sockets on $Port. Determined=$false means nothing could be read at all, +# and the caller must fail closed rather than assume the port is free of a foreign server. +# +# Socket EXISTENCE and PID ATTRIBUTION are deliberately separate. Existence comes from the kernel's +# TCP table, which lists every listener regardless of the owning account; attribution (a process +# name for the diagnostic) can fail for a socket owned by another user or by SYSTEM, so an +# unattributable owner must never read as "nothing is listening". +function Get-PortListenerInventory { + param([int]$Port) + $owners = New-Object System.Collections.Generic.List[string] + $determined = $false + $listening = $false + try { + $connections = @(Get-NetTCPConnection -State Listen -LocalPort $Port -ErrorAction Stop) + $determined = $true + foreach ($connection in $connections) { + $listening = $true + $owners.Add((Format-ListenerOwner -OwnerPid ([int]$connection.OwningProcess))) + } + } catch [System.Management.Automation.CommandNotFoundException] { + $determined = $false + } catch { + # Get-NetTCPConnection throws ObjectNotFound when nothing matches the port filter. + if ($_.CategoryInfo.Category -eq [System.Management.Automation.ErrorCategory]::ObjectNotFound) { + $determined = $true + } + } + if (-not $determined) { + $netstat = Join-Path $env:SystemRoot "System32/netstat.exe" + if (Test-Path -LiteralPath $netstat -PathType Leaf) { + try { + $rows = & $netstat -ano -p TCP 2>$null + if ($LASTEXITCODE -eq 0) { + $determined = $true + foreach ($row in $rows) { + $fields = ($row -split '\s+') | Where-Object { $_ -ne "" } + if ($fields.Count -lt 5) { continue } + if ($fields[0] -ne "TCP" -or $fields[3] -ne "LISTENING") { continue } + if ($fields[1] -notmatch ":(\d+)$" -or [int]$Matches[1] -ne $Port) { continue } + $listening = $true + $ownerPid = 0 + if ([int]::TryParse($fields[4], [ref]$ownerPid)) { + $owners.Add((Format-ListenerOwner -OwnerPid $ownerPid)) + } + } + } + } catch { $determined = $false } + } + } + return [pscustomobject]@{ + Determined = $determined + Listening = $listening + Owners = @($owners | Sort-Object -Unique) + } +} + +function Format-ListenerOwner { + param([int]$OwnerPid) + if ($OwnerPid -le 0) { return "PID $OwnerPid" } + $owner = Get-Process -Id $OwnerPid -ErrorAction SilentlyContinue + if ($null -ne $owner) { return "PID $OwnerPid ($($owner.ProcessName))" } + return "PID $OwnerPid" +} + +# Waits on an elapsed-time deadline, not a fixed iteration count: a loaded hosted runner can hold a +# listening socket well past a short fixed budget after taskkill (#1898/#2378, same family as the +# #2157 marker budget). Once the deadline passes, a port with no listening socket at all is a +# lingering kernel socket behind our own confirmed-dead tree, so it counts as released. Any +# surviving listener - and any port whose sockets cannot be inventoried - fails closed. function Wait-PortRelease { param([int]$Port) - for ($attempt = 0; $attempt -lt 50; $attempt++) { + $timeoutMs = Get-PortReleaseTimeoutMs + $stopwatch = [System.Diagnostics.Stopwatch]::StartNew() + while ($true) { if (Test-PortBindable -Port $Port) { return $true } - Start-Sleep -Milliseconds 100 + if ($stopwatch.ElapsedMilliseconds -ge $timeoutMs) { break } + Start-Sleep -Milliseconds 250 } - return $false + $elapsed = [int]$stopwatch.ElapsedMilliseconds + $inventory = Get-PortListenerInventory -Port $Port + if (-not $inventory.Determined) { + Write-DevWarning "Port $Port stayed unbindable for $elapsed ms and its listening sockets could not be inventoried." + return $false + } + if ($inventory.Listening) { + if ($inventory.Owners.Count -gt 0) { + Write-DevWarning "Port $Port is still held by a live listener ($($inventory.Owners -join ', ')) after $elapsed ms." + } else { + Write-DevWarning "Port $Port is still held by a live listener that could not be attributed to a process after $elapsed ms." + } + return $false + } + Write-Info "Port $Port stayed unbindable for $elapsed ms but nothing is listening on it; treating the lingering socket as released." + return $true } function Find-SafeApiPort { diff --git a/scripts/dev-up.sh b/scripts/dev-up.sh index 809d21ef0..39c908c37 100755 --- a/scripts/dev-up.sh +++ b/scripts/dev-up.sh @@ -370,10 +370,98 @@ port_is_bindable() { ' "$port" >/dev/null 2>&1 } +DEFAULT_PORT_RELEASE_TIMEOUT_MS=30000 + +port_release_timeout_ms() { + local raw="${TASKDECK_DEV_UP_PORT_RELEASE_TIMEOUT_MS:-}" + if [[ -n "$raw" ]]; then + # Bounded to int32 so the deadline arithmetic matches the PowerShell launcher's [int] parse and + # cannot wrap on a 64-bit shell. + if [[ "$raw" =~ ^[0-9]{1,10}$ ]] && (( 10#$raw <= 2147483647 )); then + printf '%s\n' "$(( 10#$raw ))" + return 0 + fi + warn "Ignoring invalid TASKDECK_DEV_UP_PORT_RELEASE_TIMEOUT_MS '$raw'; using ${DEFAULT_PORT_RELEASE_TIMEOUT_MS} ms." + fi + printf '%s\n' "$DEFAULT_PORT_RELEASE_TIMEOUT_MS" +} + +now_ms() { + local raw + raw="$(date +%s%3N 2>/dev/null || true)" + if [[ "$raw" =~ ^[0-9]+$ ]]; then printf '%s\n' "$raw"; return 0; fi + printf '%s000\n' "$(date +%s)" +} + +# Inventory of TCP LISTEN sockets on $1. First stdout line is "listening" or "free"; any further +# lines are attributable owner PIDs. Exit 2 means nothing could be read at all. +# +# Socket EXISTENCE and PID ATTRIBUTION are deliberately separate. An unprivileged `lsof -t` cannot +# see another user's socket at all, and `ss -p` prints the row but omits `users:(...)`, so an empty +# owner list is NOT evidence that the port is free - a root-owned listener (docker-proxy, a systemd +# unit on 5000/5173) would look ownerless. Existence therefore comes from a source that reads +# /proc/net/tcp for every user (`ss -ltn`, or `netstat -an` on BSD/macOS); attribution is +# best-effort and only decorates the diagnostic. +port_listener_inventory() { + local port="$1" rows="" all="" pids="" pid + if command -v ss >/dev/null 2>&1; then + rows="$(ss -ltnH "sport = :$port" 2>/dev/null)" || return 2 + elif command -v netstat >/dev/null 2>&1; then + all="$(netstat -an -p tcp 2>/dev/null)" || return 2 + rows="$(printf '%s\n' "$all" | grep -E "[:.]${port}[[:space:]]" | grep -i 'LISTEN' || true)" + else + return 2 + fi + rows="$(printf '%s' "$rows" | tr -d '[:space:]')" + if [[ -z "$rows" ]]; then printf 'free\n'; return 0; fi + printf 'listening\n' + if command -v ss >/dev/null 2>&1; then + pids="$(ss -ltnHp "sport = :$port" 2>/dev/null | grep -oE 'pid=[0-9]+' | cut -d= -f2 || true)" + fi + if [[ -z "$pids" ]] && command -v lsof >/dev/null 2>&1; then + pids="$(lsof -nP -iTCP:"$port" -sTCP:LISTEN -t 2>/dev/null || true)" + fi + for pid in $(printf '%s\n' "$pids" | sort -u); do + [[ "$pid" =~ ^[1-9][0-9]*$ ]] || continue + kill -0 "$pid" 2>/dev/null && printf '%s\n' "$pid" + done + return 0 +} + +# Elapsed-time deadline, not a fixed iteration count: a loaded runner can hold a listening socket +# past a short fixed budget after the tree is reaped (#1898/#2378). After the deadline, a port with +# no listening socket at all is a lingering kernel socket behind our own confirmed-dead tree and +# counts as released. Any surviving listener - and any port whose sockets cannot be inventoried - +# fails closed. wait_for_port_release() { - local port="$1" - for _ in {1..50}; do port_is_bindable "$port" && return 0; sleep 0.1; done - return 1 + local port="$1" timeout_ms status=0 inventory state owners + timeout_ms="$(port_release_timeout_ms)" + local started_at deadline elapsed + started_at="$(now_ms)" + deadline=$(( started_at + timeout_ms )) + while true; do + port_is_bindable "$port" && return 0 + (( $(now_ms) >= deadline )) && break + sleep 0.25 + done + elapsed=$(( $(now_ms) - started_at )) + inventory="$(port_listener_inventory "$port")" || status=$? + if [[ "$status" -ne 0 ]]; then + warn "Port $port stayed unbindable for ${elapsed} ms and its listening sockets could not be inventoried." + return 1 + fi + state="$(printf '%s\n' "$inventory" | head -n 1)" + owners="$(printf '%s\n' "$inventory" | tail -n +2 | tr '\n' ' ' | sed 's/ */ /g; s/^ //; s/ $//')" + if [[ "$state" != "free" ]]; then + if [[ -n "$owners" ]]; then + warn "Port $port is still held by a live listener (PID $owners) after ${elapsed} ms." + else + warn "Port $port is still held by a live listener that could not be attributed to a PID after ${elapsed} ms." + fi + return 1 + fi + info "Port $port stayed unbindable for ${elapsed} ms but nothing is listening on it; treating the lingering socket as released." + return 0 } find_safe_api_port() {