Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 24 additions & 12 deletions packages/service-manager/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -126,18 +126,30 @@ async function runChecked(
runner: ServiceRunner,
command: readonly string[],
): Promise<ServiceRunnerResult> {
let result: ServiceRunnerResult;
try {
result = await runner.run(command);
} catch (cause) {
throw new ServiceCommandError(command, {
exitCode: null,
stdout: "",
stderr: sanitizeDiagnostic(String(cause)),
});
}
if (result.exitCode !== 0) throw new ServiceCommandError(command, result);
return result;
const isLaunchctlBootstrap = command[0] === "launchctl" && command[1] === "bootstrap";
// launchd can accept bootout before it has finished removing the service.
// A replacement bootstrap then returns EINPROGRESS (37) for a short window.
const retryDeadline = Date.now() + 3_000;
while (true) {
let result: ServiceRunnerResult;
try {
result = await runner.run(command);
} catch (cause) {
throw new ServiceCommandError(command, {
exitCode: null,
stdout: "",
stderr: sanitizeDiagnostic(String(cause)),
});
}
if (result.exitCode === 0) return result;
const diagnostics = `${result.stdout}\n${result.stderr}`;
const removalStillFinishing =
isLaunchctlBootstrap &&
(result.exitCode === 37 || /operation already in progress|bootstrap failed:\s*37\b/iu.test(diagnostics));
if (!removalStillFinishing || Date.now() >= retryDeadline)
throw new ServiceCommandError(command, result);
await new Promise<void>((resolve) => setTimeout(resolve, 100));
}
}

abstract class BaseManager implements ServiceManager {
Expand Down
20 changes: 20 additions & 0 deletions packages/service-manager/test/service-manager.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -346,6 +346,26 @@ describe("service-manager lifecycle", () => {
await manager.restart();
expect(runner.calls[1]).toEqual(["launchctl", "bootstrap", "gui/501", manager.definitionPath]);
});
it("waits for launchd to finish removing an upgraded Mac agent before bootstrap", async () => {
const fs = new MemoryFs();
const runner = new MemoryRunner();
const manager = new MacLaunchAgentManager(spec, { ...options(fs, runner), uid: 501 });
fs.files.set(manager.definitionPath, "old definition");
runner.results = [
{ exitCode: 0, stdout: "state = running\n", stderr: "" },
{ exitCode: 0, stdout: "", stderr: "" },
{ exitCode: 37, stdout: "", stderr: "Bootstrap failed: 37: Operation already in progress" },
{ exitCode: 0, stdout: "", stderr: "" },
{ exitCode: 0, stdout: "", stderr: "" },
];

await manager.install();

expect(
runner.calls.filter((call) => call[0] === "launchctl" && call[1] === "bootstrap"),
).toHaveLength(2);
expect(fs.files.get(manager.definitionPath)).toBe(renderMacLaunchAgentDefinition(spec));
});
it("rejects unsupported executable argv and never emits a canary", () => {
const canary = "API_TOKEN_CANARY";
expect(
Expand Down
26 changes: 25 additions & 1 deletion scripts/tailnet-service.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -397,7 +397,12 @@ function sanitizedOutput(value) {
.slice(0, 2_048);
}

async function runCommand(command) {
export function isTransientLaunchctlBootstrap(command, result) {
if (command.argv[0] !== "launchctl" || command.argv[1] !== "bootstrap") return false;
return result.code === 37 || /operation already in progress|bootstrap failed:\s*37\b/iu.test(`${result.stdout}\n${result.stderr}`);
}

async function runCommandOnce(command) {
return await new Promise((resolvePromise, reject) => {
const [executable, ...args] = command.argv;
const child = spawn(executable, args, { shell: false, stdio: ["ignore", "pipe", "pipe"] });
Expand Down Expand Up @@ -425,6 +430,25 @@ async function runCommand(command) {
});
}

async function runCommand(command) {
// macOS service removal is asynchronous even after `launchctl bootout` exits.
// Retry only launchd's explicit EINPROGRESS response during replacement.
const retryDeadline = Date.now() + 3_000;
while (true) {
const result = await runCommandOnce({ ...command, allowFailure: true });
if (result.code === 0) return result;
if (isTransientLaunchctlBootstrap(command, result) && Date.now() < retryDeadline) {
await new Promise((resolvePromise) => setTimeout(resolvePromise, 100));
continue;
}
if (command.allowFailure !== true) {
const executable = command.argv[0];
throw new Error(`${executable} exited ${result.code}: ${result.stderr || result.stdout || "no diagnostics"}`);
}
return result;
}
}

async function runCommands(commands) {
const results = [];
for (const command of commands) results.push(await runCommand(command));
Expand Down
24 changes: 24 additions & 0 deletions scripts/tailnet-service.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { test } from "node:test";
import {
DEFAULT_GATEWAY_PORT,
SERVICE_LABEL,
isTransientLaunchctlBootstrap,
parseCli,
parseProfileRoutesOption,
renderLaunchAgent,
Expand Down Expand Up @@ -258,6 +259,29 @@ test("supervisor command plans never use a shell and include durable enablement"
}
});

test("only launchctl bootstrap's asynchronous-removal error is retryable", () => {
const bootstrap = { argv: ["launchctl", "bootstrap", "gui/501", "/tmp/service.plist"] };
assert.equal(
isTransientLaunchctlBootstrap(bootstrap, {
code: 37,
stdout: "",
stderr: "Bootstrap failed: 37: Operation already in progress",
}),
true,
);
assert.equal(
isTransientLaunchctlBootstrap(bootstrap, { code: 5, stdout: "", stderr: "Input/output error" }),
false,
);
assert.equal(
isTransientLaunchctlBootstrap(
{ argv: ["launchctl", "kickstart", "gui/501/example"] },
{ code: 37, stdout: "", stderr: "Operation already in progress" },
),
false,
);
});

test("CLI parser rejects ambiguous values and accepts the documented install shape", () => {
assert.deepEqual(parseCli(["--help"]), { command: "help", options: {} });
assert.deepEqual(
Expand Down
Loading