From e45bd73d6e5c9aeb5cc6ba3eaab15d2ed6c0e8ac Mon Sep 17 00:00:00 2001 From: ryanngit Date: Thu, 23 Jul 2026 16:17:12 +0700 Subject: [PATCH] fix(tunnel): preserve successor cloudflared PID Make PID cleanup conditional on the exiting child still owning the PID file so a stale exit cannot erase a replacement tunnel's PID. Only null the in-memory process when the exiting child is current. Explicit disable keeps unconditional cleanup. --- src/lib/tunnel/cloudflare/cloudflared.js | 8 ++--- src/lib/tunnel/cloudflare/pid.js | 8 +++-- tests/unit/tunnel-pid-ownership.test.js | 39 ++++++++++++++++++++++++ 3 files changed, 49 insertions(+), 6 deletions(-) create mode 100644 tests/unit/tunnel-pid-ownership.test.js diff --git a/src/lib/tunnel/cloudflare/cloudflared.js b/src/lib/tunnel/cloudflare/cloudflared.js index 38e4bf74..09458fc5 100644 --- a/src/lib/tunnel/cloudflare/cloudflared.js +++ b/src/lib/tunnel/cloudflare/cloudflared.js @@ -239,8 +239,8 @@ export async function spawnCloudflared(tunnelToken) { }); child.on("exit", (code, signal) => { - cloudflaredProcess = null; - clearPid(); + if (cloudflaredProcess === child) cloudflaredProcess = null; + clearPid(child.pid); const wasConnected = resolved; // true = already connected successfully if (!resolved) { resolved = true; @@ -372,8 +372,8 @@ export async function spawnQuickTunnel(localPort, onUrlUpdate) { }); child.on("exit", (code, signal) => { - cloudflaredProcess = null; - clearPid(); + if (cloudflaredProcess === child) cloudflaredProcess = null; + clearPid(child.pid); // Deliberate kill (restart/disable) — exit silently, no error noise if (intentionalKill) { intentionalKill = false; diff --git a/src/lib/tunnel/cloudflare/pid.js b/src/lib/tunnel/cloudflare/pid.js index 919837c5..77c63a82 100644 --- a/src/lib/tunnel/cloudflare/pid.js +++ b/src/lib/tunnel/cloudflare/pid.js @@ -16,8 +16,12 @@ export function loadPid() { return null; } -export function clearPid() { +export function clearPid(expectedPid = null) { try { - if (fs.existsSync(PID_FILE)) fs.unlinkSync(PID_FILE); + if (!fs.existsSync(PID_FILE)) return false; + if (expectedPid !== null && loadPid() !== expectedPid) return false; + fs.unlinkSync(PID_FILE); + return true; } catch { /* ignore */ } + return false; } diff --git a/tests/unit/tunnel-pid-ownership.test.js b/tests/unit/tunnel-pid-ownership.test.js new file mode 100644 index 00000000..57bc2bd0 --- /dev/null +++ b/tests/unit/tunnel-pid-ownership.test.js @@ -0,0 +1,39 @@ +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +describe("cloudflared PID ownership", () => { + let dataDir; + + beforeEach(() => { + dataDir = fs.mkdtempSync(path.join(os.tmpdir(), "9router-tunnel-pid-")); + process.env.DATA_DIR = dataDir; + vi.resetModules(); + }); + + afterEach(() => { + delete process.env.DATA_DIR; + fs.rmSync(dataDir, { recursive: true, force: true }); + }); + + it("does not let an old child clear its successor PID", async () => { + const { clearPid, loadPid, savePid } = await import("../../src/lib/tunnel/cloudflare/pid.js"); + + savePid(100); + savePid(200); + clearPid(100); + + expect(loadPid()).toBe(200); + + clearPid(200); + expect(loadPid()).toBeNull(); + }); + + it("releases PID and process ownership for the exiting child only", () => { + const source = fs.readFileSync(new URL("../../src/lib/tunnel/cloudflare/cloudflared.js", import.meta.url), "utf8"); + + expect(source.match(/clearPid\(child\.pid\)/g)).toHaveLength(2); + expect(source.match(/cloudflaredProcess === child/g)).toHaveLength(2); + }); +});