Repository navigation
fix(devices): safely reclaim obsolete managed tool versions #12819
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
2b05a84
fix(devices): prune unused managed tool versions after successful sta…
juliusmarminge 9f4f9f8
fix(devices): distinguish reused PIDs during tool maintenance
juliusmarminge ee05007
fix(devices): normalize process identity across server locales
juliusmarminge 5f06601
fix(devices): retain context in tool maintenance failures
juliusmarminge 9325e50
fix(devices): preserve missing maintenance exit statuses
juliusmarminge 65eebc8
fix(devices): reclaim maintenance locks without deleting new owners
juliusmarminge c66c4b9
fix(devices): remove advisory install leases from startup
juliusmarminge File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,159 @@ | ||
| import * as Effect from "effect/Effect"; | ||
| import * as NodePathLayer from "@effect/platform-node/NodePath"; | ||
| import * as ProcessRunner from "../processRunner.ts"; | ||
| import * as ChildProcessSpawner from "effect/unstable/process/ChildProcessSpawner"; | ||
| // @effect-diagnostics nodeBuiltinImport:off - tests the same standalone script used by local and SSH hosts. | ||
| import { describe, expect, it } from "@effect/vitest"; | ||
| import * as NodeFSP from "node:fs/promises"; | ||
| import * as NodeOS from "node:os"; | ||
| import * as NodePath from "node:path"; | ||
| import * as NodeChildProcess from "node:child_process"; | ||
| import * as NodeUtil from "node:util"; | ||
| import { pruneLocalDeviceTools, deviceToolMaintenanceScript } from "./deviceToolMaintenance.ts"; | ||
|
|
||
| const exec = NodeUtil.promisify(NodeChildProcess.execFile); | ||
|
|
||
| describe.each([false, true])("device tool cleanup, flat=%s", (flat) => { | ||
| it("keeps current, previous, active and incomplete installs, pruning unused completed versions", async () => { | ||
| const root = await NodeFSP.mkdtemp(NodePath.join(NodeOS.tmpdir(), "t3-tool-cleanup-")); | ||
| const name = "expo-device-hub"; | ||
| const directory = (version: string) => | ||
| flat ? NodePath.join(root, `${name}@${version}`) : NodePath.join(root, name, version); | ||
| try { | ||
| for (const version of ["0.1.0", "0.2.0", "0.3.0", "0.4.0", "0.5.0", "0.6.0"]) { | ||
| await NodeFSP.mkdir(directory(version), { recursive: true }); | ||
| if (version === "0.5.0") continue; | ||
| const sentinel = NodePath.join(directory(version), ".install-complete"); | ||
| await NodeFSP.writeFile(sentinel, version); | ||
| await NodeFSP.utimes( | ||
| sentinel, | ||
| Number(version.split(".")[1]), | ||
| Number(version.split(".")[1]), | ||
| ); | ||
| } | ||
| const script = | ||
| deviceToolMaintenanceScript + | ||
| ` | ||
| (async () => { | ||
| const root = ${JSON.stringify(root)}; | ||
| await pruneTools(root, [['${name}', '0.6.0']], ${flat}); | ||
| })().catch(error => { console.error(error); process.exitCode = 1; });`; | ||
| await exec(process.execPath, [ | ||
| "-e", | ||
| script, | ||
| NodePath.join(directory("0.2.0"), "active-helper.cjs"), | ||
| ]); | ||
| await expect(NodeFSP.stat(directory("0.1.0"))).rejects.toThrow(); | ||
| await expect(NodeFSP.stat(directory("0.3.0"))).rejects.toThrow(); | ||
| for (const version of ["0.2.0", "0.4.0", "0.5.0", "0.6.0"]) | ||
| expect((await NodeFSP.stat(directory(version))).isDirectory()).toBe(true); | ||
| } finally { | ||
| await NodeFSP.rm(root, { recursive: true, force: true }); | ||
| } | ||
| }); | ||
|
|
||
| it("keeps every install when the process scan fails", async () => { | ||
| const root = await NodeFSP.mkdtemp(NodePath.join(NodeOS.tmpdir(), "t3-tool-scan-")); | ||
| try { | ||
| for (const version of ["0.1.0", "0.2.0", "0.3.0"]) { | ||
| const dir = flat | ||
| ? NodePath.join(root, `expo-device-hub@${version}`) | ||
| : NodePath.join(root, "expo-device-hub", version); | ||
| await NodeFSP.mkdir(dir, { recursive: true }); | ||
| await NodeFSP.writeFile(NodePath.join(dir, ".install-complete"), version); | ||
| } | ||
| await exec(process.execPath, [ | ||
| "-e", | ||
| deviceToolMaintenanceScript + | ||
| ` | ||
| require('node:child_process').spawnSync = () => ({ status: 1, stdout: '' }); | ||
| pruneTools(${JSON.stringify(root)}, [['expo-device-hub', '0.3.0']], ${flat}).catch(() => process.exitCode = 1); | ||
| `, | ||
| ]); | ||
| const parent = flat ? root : NodePath.join(root, "expo-device-hub"); | ||
| expect((await NodeFSP.readdir(parent)).length).toBe(3); | ||
| } finally { | ||
| await NodeFSP.rm(root, { recursive: true, force: true }); | ||
| } | ||
| }); | ||
|
|
||
| it("does not prune before the required version has completed installation", async () => { | ||
| const root = await NodeFSP.mkdtemp(NodePath.join(NodeOS.tmpdir(), "t3-tool-cleanup-")); | ||
| try { | ||
| const dir = flat | ||
| ? NodePath.join(root, "expo-device-hub@0.1.0") | ||
| : NodePath.join(root, "expo-device-hub/0.1.0"); | ||
| await NodeFSP.mkdir(dir, { recursive: true }); | ||
| await NodeFSP.writeFile(NodePath.join(dir, ".install-complete"), "0.1.0"); | ||
| await exec(process.execPath, [ | ||
| "-e", | ||
| deviceToolMaintenanceScript + | ||
| `pruneTools(${JSON.stringify(root)}, [['expo-device-hub','0.6.0']], ${flat}).catch(() => process.exitCode = 1);`, | ||
| ]); | ||
| expect((await NodeFSP.stat(dir)).isDirectory()).toBe(true); | ||
| } finally { | ||
| await NodeFSP.rm(root, { recursive: true, force: true }); | ||
| } | ||
| }); | ||
| }); | ||
|
|
||
| it.effect("maintenance failures retain safe context and the original process result", () => | ||
| Effect.gen(function* () { | ||
| const output = { | ||
| code: ChildProcessSpawner.ExitCode(1), | ||
| stdout: "", | ||
| stderr: "private child diagnostics", | ||
| timedOut: false, | ||
| stdoutTruncated: false, | ||
| stderrTruncated: false, | ||
| stdoutInvalidUtf8: false, | ||
| stderrInvalidUtf8: false, | ||
| }; | ||
| for (const [operation, run] of [["prune", pruneLocalDeviceTools]] as const) { | ||
| const error = yield* run("/tools", process.execPath, "hub").pipe( | ||
| Effect.provideService(ProcessRunner.ProcessRunner, { run: () => Effect.succeed(output) }), | ||
| Effect.flip, | ||
| ); | ||
| expect(error).toMatchObject({ | ||
| _tag: "DeviceToolMaintenanceError", | ||
| operation, | ||
| tool: "hub", | ||
| exitCode: 1, | ||
| cause: output, | ||
| }); | ||
| expect(error.message).toBe(`Device tool ${operation} failed for hub (exit code 1).`); | ||
| expect(error.message).not.toContain(output.stderr); | ||
| } | ||
| }).pipe(Effect.provide(NodePathLayer.layer)), | ||
| ); | ||
|
|
||
| it("serializes competing maintenance processes after reclaiming a stale lock", async () => { | ||
| const root = await NodeFSP.mkdtemp(NodePath.join(NodeOS.tmpdir(), "t3-tool-contention-")); | ||
| try { | ||
| const lock = NodePath.join(root, ".maintenance-lock"); | ||
| await NodeFSP.mkdir(lock); | ||
| await NodeFSP.writeFile( | ||
| NodePath.join(lock, "stale-owner.json"), | ||
| JSON.stringify({ pid: 2147483647, identity: "dead" }), | ||
| ); | ||
| const script = | ||
| deviceToolMaintenanceScript + | ||
| ` | ||
| (async () => { | ||
| const root = ${JSON.stringify(root)}; | ||
| const marker = maintenancePath.join(root, 'critical-section'); | ||
| for (let attempt = 0; attempt < 8; attempt++) await withToolMaintenance(root, () => { | ||
| maintenanceFs.writeFileSync(marker, String(process.pid), { flag: 'wx' }); | ||
| for (let check = 0; check < 100; check++) { | ||
| if (maintenanceFs.readFileSync(marker, 'utf8') !== String(process.pid)) throw Error('Overlapping maintenance'); | ||
| } | ||
| maintenanceFs.unlinkSync(marker); | ||
| }); | ||
| })().catch(error => { console.error(error); process.exitCode = 1; });`; | ||
| await Promise.all(Array.from({ length: 6 }, () => exec(process.execPath, ["-e", script]))); | ||
| await expect(NodeFSP.stat(lock)).rejects.toThrow(); | ||
| expect(await NodeFSP.readdir(root)).toEqual([]); | ||
| } finally { | ||
| await NodeFSP.rm(root, { recursive: true, force: true }); | ||
| } | ||
| }); |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,148 @@ | ||
| // @effect-diagnostics preferSchemaOverJson:off - JSON string literals safely embed paths and arguments in generated JavaScript. | ||
| import * as Schema from "effect/Schema"; | ||
| import * as Effect from "effect/Effect"; | ||
| import * as Path from "effect/Path"; | ||
| import * as ProcessRunner from "../processRunner.ts"; | ||
| import { AGENT_DEVICE_VERSION, DEVICE_HUB_VERSION } from "./DeviceToolchain.ts"; | ||
|
|
||
| /** Shared with the SSH bootstrap. Cleanup runs only after successful startup. */ | ||
| export const deviceToolMaintenanceScript = String.raw` | ||
| const maintenanceFs = require('node:fs'); | ||
| const maintenancePath = require('node:path'); | ||
| const maintenanceAlive = pid => { | ||
| try { process.kill(pid, 0); return true; } | ||
| catch (error) { return error.code !== 'ESRCH'; } | ||
| }; | ||
| async function withToolMaintenance(root, operation) { | ||
| maintenanceFs.mkdirSync(root, { recursive: true }); | ||
| const lock = maintenancePath.join(root, '.maintenance-lock'); | ||
| const nonce = require('node:crypto').randomUUID(); | ||
| const ownerFile = process.pid + '.' + nonce + '.json'; | ||
| const candidate = lock + '.' + nonce; | ||
| const holder = { pid: process.pid }; | ||
| const deadline = Date.now() + 30000; | ||
| const removeEmptyLock = () => { | ||
| try { maintenanceFs.rmdirSync(lock); } | ||
| catch (error) { if (!['ENOENT', 'ENOTEMPTY', 'EEXIST', 'EPERM'].includes(error.code)) throw error; } | ||
| }; | ||
| maintenanceFs.mkdirSync(candidate); | ||
| try { | ||
| maintenanceFs.writeFileSync(maintenancePath.join(candidate, ownerFile), JSON.stringify(holder)); | ||
| while (true) { | ||
| try { | ||
| // Publish a populated directory atomically; rename cannot replace another populated lock. | ||
| maintenanceFs.renameSync(candidate, lock); | ||
| break; | ||
| } | ||
| catch (error) { | ||
| if (!['EEXIST', 'ENOTEMPTY', 'EPERM', 'EACCES'].includes(error.code)) throw error; | ||
| let files = []; | ||
| try { files = maintenanceFs.readdirSync(lock); } catch (error) { if (error.code !== 'ENOENT') throw error; } | ||
| if (files.length === 1) { | ||
| const previousFile = maintenancePath.join(lock, files[0]); | ||
| let previous; | ||
| try { previous = JSON.parse(maintenanceFs.readFileSync(previousFile, 'utf8')); } catch {} | ||
| if (Number.isSafeInteger(previous?.pid) && previous.pid > 0 && !maintenanceAlive(previous.pid)) { | ||
| // The unique filename belongs only to that owner. Never unlink a replacement owner's file. | ||
| try { maintenanceFs.unlinkSync(previousFile); } catch (error) { if (error.code !== 'ENOENT') throw error; } | ||
| } | ||
| } | ||
| // A concurrent acquirer publishes its owner file with the directory, so this cannot remove it. | ||
| removeEmptyLock(); | ||
| if (Date.now() >= deadline) throw Error('Device tool maintenance is locked. Retry when the other operation finishes.'); | ||
| await new Promise(resolve => setTimeout(resolve, 50)); | ||
| } | ||
| } | ||
| try { return operation(); } | ||
| finally { | ||
| maintenanceFs.unlinkSync(maintenancePath.join(lock, ownerFile)); | ||
| removeEmptyLock(); | ||
| } | ||
| } finally { | ||
| maintenanceFs.rmSync(candidate, { recursive: true, force: true }); | ||
| } | ||
| } | ||
| function pruneTools(root, specs, flat) { | ||
| return withToolMaintenance(root, () => { | ||
| // Keep installs used by any running helper, including older T3 releases. | ||
| const scan = process.platform === 'win32' | ||
| ? require('node:child_process').spawnSync('powershell.exe', ['-NoProfile', '-NonInteractive', '-Command', 'Get-CimInstance Win32_Process | Select-Object -ExpandProperty CommandLine'], { encoding: 'utf8', timeout: 10000 }) | ||
| : require('node:child_process').spawnSync('ps', ['-ax', '-o', 'command='], { encoding: 'utf8', timeout: 10000 }); | ||
| if (scan.status !== 0 || !scan.stdout) return; | ||
| for (const [name, required] of specs) { | ||
| const parent = flat ? root : maintenancePath.join(root, name); | ||
| let names; | ||
| try { names = maintenanceFs.readdirSync(parent); } catch { continue; } | ||
| const completed = []; | ||
| for (const item of names) { | ||
| const version = flat ? (item.startsWith(name + '@') ? item.slice(name.length + 1) : '') : item; | ||
| if (!/^[0-9]+\.[0-9]+\.[0-9]+(?:-[a-zA-Z0-9.-]+)?$/.test(version)) continue; | ||
| const directory = maintenancePath.join(parent, item); | ||
| try { | ||
| if (!maintenanceFs.lstatSync(directory).isDirectory()) continue; | ||
| if (maintenanceFs.readFileSync(maintenancePath.join(directory, '.install-complete'), 'utf8').trim() !== version) continue; | ||
| completed.push({ version, directory, modified: maintenanceFs.statSync(maintenancePath.join(directory, '.install-complete')).mtimeMs }); | ||
| } catch {} | ||
| } | ||
| // Never prune until the required install has completed. Retain the last other successful install. | ||
| if (!completed.some(value => value.version === required)) continue; | ||
| const previous = completed.filter(value => value.version !== required).sort((a, b) => b.modified - a.modified || b.version.localeCompare(a.version, 'en', { numeric: true }))[0]?.version; | ||
| for (const { version, directory } of completed) { | ||
| if (version === required || version === previous || scan.stdout.includes(directory + maintenancePath.sep)) continue; | ||
| maintenanceFs.rmSync(directory, { recursive: true, force: true }); | ||
| } | ||
| } | ||
| }); | ||
| } | ||
| `; | ||
|
|
||
| class DeviceToolMaintenanceError extends Schema.TaggedError<DeviceToolMaintenanceError>()( | ||
| "DeviceToolMaintenanceError", | ||
| { | ||
| operation: Schema.Literal("prune"), | ||
| tool: Schema.Literals(["hub", "agent"]), | ||
| exitCode: Schema.NullOr(Schema.Int), | ||
| cause: Schema.Defect(), | ||
| }, | ||
| ) { | ||
| override get message() { | ||
| return `Device tool ${this.operation} failed for ${this.tool} (exit code ${this.exitCode ?? "unknown"}).`; | ||
| } | ||
| } | ||
|
|
||
| const runMaintenance = Effect.fn("DeviceToolchain.maintenance")(function* ( | ||
| nodePath: string, | ||
| script: string, | ||
| operation: "prune", | ||
| tool: "hub" | "agent", | ||
| ) { | ||
| const runner = yield* ProcessRunner.ProcessRunner; | ||
| const result = yield* runner.run({ | ||
| command: nodePath, | ||
| args: [ | ||
| "-e", | ||
| deviceToolMaintenanceScript + | ||
| "\n" + | ||
| script + | ||
| ".catch(error => { console.error(error.message); process.exitCode = 1; });", | ||
| ], | ||
| }); | ||
| if (result.code !== 0) | ||
| return yield* Effect.fail( | ||
| new DeviceToolMaintenanceError({ operation, tool, exitCode: result.code, cause: result }), | ||
| ); | ||
| }); | ||
|
|
||
| export const pruneLocalDeviceTools = Effect.fn("DeviceToolchain.prune")(function* ( | ||
| baseDir: string, | ||
| nodePath: string, | ||
| tool: "hub" | "agent", | ||
| ) { | ||
| const path = yield* Path.Path; | ||
| yield* runMaintenance( | ||
| nodePath, | ||
| `pruneTools(${JSON.stringify(path.join(baseDir, "tools"))}, ${JSON.stringify(tool === "hub" ? [["expo-device-hub", DEVICE_HUB_VERSION]] : [["agent-device", AGENT_DEVICE_VERSION]])}, false)`, | ||
| "prune", | ||
| tool, | ||
| ); | ||
| }); |
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
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
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.