diff --git a/packages/ssh/src/tunnel.test.ts b/packages/ssh/src/tunnel.test.ts index 692f105e4a57..fb80b1f37176 100644 --- a/packages/ssh/src/tunnel.test.ts +++ b/packages/ssh/src/tunnel.test.ts @@ -124,8 +124,6 @@ describe("ssh tunnel scripts", () => { script, 'T3_LOCK="$HOME/.t3/runtime/versions/.$T3_ARCHIVE_VERSION.install.lock"', ); - assert.include(script, 'while ! mkdir "$T3_LOCK" 2>/dev/null; do'); - assert.include(script, 'mv "$T3_LOCK/pid.tmp" "$T3_LOCK/pid"'); assert.include(script, 'if ! kill -0 "$T3_LOCK_OWNER" 2>/dev/null; then'); assert.include(script, 'if [ "$T3_LOCK_UNOWNED" -ge 5 ]; then'); assert.include(script, 'if [ "$T3_LOCK_WAITED" -ge 360 ]; then'); @@ -233,7 +231,6 @@ describe("ssh tunnel scripts", () => { it("embeds the runner in the launch script byte for byte", () => { const runner = SshTunnel.buildRemoteT3RunnerScript(ARCHIVE); - assert.include(runner, 'printf \'%s\\n\' "$$" > "$T3_LOCK/pid.tmp"'); assert.include(SshTunnel.buildRemoteLaunchScript(ARCHIVE), runner); }); @@ -1053,13 +1050,22 @@ describe("remote port picker", () => { ); }); -const runScript = (home: string, args: ReadonlyArray) => +const runScript = ( + home: string, + args: ReadonlyArray, + options?: { + path?: string; + stdin?: Stream.Stream; + onStderrLine?: (line: string) => Effect.Effect; + }, +) => Effect.gen(function* () { const spawner = yield* ChildProcessSpawner.ChildProcessSpawner; const child = yield* spawner.spawn( ChildProcess.make("sh", args, { - env: { PATH: process.env.PATH ?? "", HOME: home }, + env: { PATH: options?.path ?? process.env.PATH ?? "", HOME: home }, extendEnv: false, + ...(options?.stdin ? { stdin: { stream: options.stdin } } : {}), }), ); const [stdout, stderr, exitCode] = yield* Effect.all( @@ -1073,9 +1079,11 @@ const runScript = (home: string, args: ReadonlyArray) => ), child.stderr.pipe( Stream.decodeText(), + Stream.splitLines, + Stream.tap((line) => options?.onStderrLine?.(line) ?? Effect.void), Stream.runFold( () => "", - (acc, chunk) => acc + chunk, + (acc, line) => `${acc}${line}\n`, ), ), child.exitCode.pipe(Effect.map(Number)), @@ -1091,7 +1099,8 @@ describe("archive runner script", () => { const windowsHost = hostPlatform === "win32"; const archiveVersion = "1.2.3-preview.20260911.4"; - const runRunner = (home: string, runner: string) => runScript(home, [runner, "--version"]); + const runRunner = (home: string, runner: string, options?: Parameters[2]) => + runScript(home, [runner, "--version"], options); const makeMirror = Effect.fn("makeMirror")(function* (root: string) { const spawner = yield* ChildProcessSpawner.ChildProcessSpawner; @@ -1127,11 +1136,45 @@ describe("archive runner script", () => { ); const home = `${root}/home`; yield* fs.makeDirectory(home, { recursive: true }); - - const results = yield* Effect.all( - [runRunner(home, runner), runRunner(home, runner), runRunner(home, runner)], - { concurrency: "unbounded" }, - ); + // Reproduce mkdir succeeding on EEXIST, even on hosts without the uutils bug. + const bin = `${root}/bin`; + yield* fs.makeDirectory(bin); + for (const [name, script] of Object.entries({ + mkdir: 'PATH=${PATH#*:}\nexec mkdir -p "$@"', + tar: 'echo >> "${0%/*}/installs"\necho installing >&2\nread -r release\nPATH=${PATH#*:}\nexec tar "$@"', + sleep: 'echo waiting >&2\nPATH=${PATH#*:}\nexec sleep "$@"', + })) { + yield* fs.writeFileString(`${bin}/${name}`, `#!/bin/sh\n${script}\n`); + yield* fs.chmod(`${bin}/${name}`, 0o755); + } + const path = `${bin}:${process.env.PATH ?? ""}`; + const installing = yield* Deferred.make(); + const contended = yield* Deferred.make(); + // Hold the first extraction until another runner either waits for the lock + // or incorrectly begins installing. This forces contention without timing assumptions. + const first = yield* runRunner(home, runner, { + path, + stdin: Stream.fromEffect( + Deferred.await(contended).pipe(Effect.as(new TextEncoder().encode("release\n"))), + ), + onStderrLine: (line) => + line === "installing" + ? Deferred.succeed(installing, undefined).pipe(Effect.asVoid) + : Effect.void, + }).pipe(Effect.forkChild); + yield* Deferred.await(installing); + const contender = runRunner(home, runner, { + path, + stdin: Stream.make(new TextEncoder().encode("release\n")), + onStderrLine: (line) => + line === "installing" || line === "waiting" + ? Deferred.succeed(contended, undefined).pipe(Effect.asVoid) + : Effect.void, + }); + const results = yield* Effect.all([Fiber.join(first), contender, contender], { + concurrency: "unbounded", + }); + assert.equal(yield* fs.readFileString(`${bin}/installs`), "\n"); for (const result of results) { assert.equal(result.exitCode, 0, result.stderr); assert.include(result.stdout, `t3 v${archiveVersion}`); @@ -1145,13 +1188,12 @@ describe("archive runner script", () => { const lock = `${versionsDir}/.${archiveVersion}.install.lock`; yield* fs.remove(`${versionsDir}/${archiveVersion}`, { recursive: true }); - yield* fs.makeDirectory(lock); - yield* fs.writeFileString(`${lock}/pid`, "999999\n"); + yield* fs.writeFileString(lock, "999999\n"); const afterDead = yield* runRunner(home, runner); assert.equal(afterDead.exitCode, 0, afterDead.stderr); yield* fs.remove(`${versionsDir}/${archiveVersion}`, { recursive: true }); - yield* fs.makeDirectory(lock); + yield* fs.writeFileString(lock, ""); const afterUnowned = yield* runRunner(home, runner); assert.equal(afterUnowned.exitCode, 0, afterUnowned.stderr); assert.isFalse(yield* fs.exists(lock)); diff --git a/packages/ssh/src/tunnel.ts b/packages/ssh/src/tunnel.ts index 11371a38f26b..c7be29a9dedb 100644 --- a/packages/ssh/src/tunnel.ts +++ b/packages/ssh/src/tunnel.ts @@ -461,21 +461,13 @@ t3_runtime_ready() { } if ! t3_runtime_ready; then mkdir -p "$HOME/.t3/runtime/versions" - # Concurrent launches (two clients, a retry racing a slow first run) must - # not both install: mkdir is the atomic lock and the ready check repeats - # under it. T3_LOCK="$HOME/.t3/runtime/versions/.$T3_ARCHIVE_VERSION.install.lock" - # mkdir is the only portable atomic exclusive create (mv would silently - # nest a candidate inside an existing lock). The owner publishes its pid - # right after, so a lock with a live owner is never reclaimed however - # slow its download is, and a lock whose owner is dead is reclaimed at - # once. A lock with no pid at all is a crash between mkdir and the pid - # write; it is reclaimed after a short grace so a live owner has time to - # publish. + # uutils mkdir can report success on EEXIST, so use shell noclobber. + # An empty lock gets a short grace period for its owner to publish a PID. T3_LOCK_WAITED=0 T3_LOCK_UNOWNED=0 - while ! mkdir "$T3_LOCK" 2>/dev/null; do - T3_LOCK_OWNER="$(cat "$T3_LOCK/pid" 2>/dev/null || true)" + while ! (set -C; printf '%s\\n' "$$" > "$T3_LOCK") 2>/dev/null; do + T3_LOCK_OWNER="$(cat "$T3_LOCK" 2>/dev/null || true)" if [ -n "$T3_LOCK_OWNER" ]; then T3_LOCK_UNOWNED=0 if ! kill -0 "$T3_LOCK_OWNER" 2>/dev/null; then @@ -496,7 +488,6 @@ if ! t3_runtime_ready; then sleep 1 T3_LOCK_WAITED=$((T3_LOCK_WAITED + 1)) done - printf '%s\\n' "$$" > "$T3_LOCK/pid.tmp" && mv "$T3_LOCK/pid.tmp" "$T3_LOCK/pid" trap 'rm -rf "$T3_LOCK"' EXIT fi if ! t3_runtime_ready; then