fix(client): wait for service shutdown before restart

packages/client/src/effect/service.ts

@@ -150,7 +150,7 @@ export const stop = Effect.fn("service.stop")(function* (options: StopOptions =
       PtyHandoff.prepare(options.file ?? fallback(), info, defaultEnsureTiming.requestTimeout),
     )
   else yield* Effect.tryPromise(() => PtyHandoff.clear(options.file ?? fallback()))
-  if (info !== undefined) yield* terminate(info, options, defaultEnsureTiming)
+  if (info !== undefined) yield* terminate(info, options, ensureTiming(options))
 })
 
 function fallback() {
@@ -291,9 +291,9 @@ const terminate = Effect.fnUntraced(function* (info: Info, options: { readonly f
   if (current === undefined || !same(current, info)) return
   yield* signal(info.pid, "SIGTERM")
   const done = yield* stopped(info.pid).pipe(Effect.retry(poll(timing)), Effect.option)
+  // A gracefully stopping server may remove its registration before it releases its port, so only
+  // the exact process we signalled can tell us when it is safe for a restart to continue.
   if (Option.isNone(done)) {
-    const latest = yield* read(options.file)
-    if (latest === undefined || !same(latest, info)) return
     yield* signal(info.pid, "SIGKILL")
     yield* stopped(info.pid).pipe(Effect.retry(poll(timing)))
   }

packages/client/src/promise/service.ts

@@ -121,7 +121,7 @@ export async function stop(options: StopOptions = {}) {
   if (options.pty === "handoff" && info !== undefined)
     await PtyHandoff.prepare(options.file ?? fallback(), info, defaultEnsureTiming.requestTimeout)
   else await PtyHandoff.clear(options.file ?? fallback())
-  if (info !== undefined) await terminate(info, options, defaultEnsureTiming)
+  if (info !== undefined) await terminate(info, options, ensureTiming(options))
 }
 
 function fallback() {
@@ -250,9 +250,9 @@ async function terminate(info: Info, options: { readonly file?: string }, timing
   const current = await read(options.file)
   if (current === undefined || !same(current, info)) return
   signal(info.pid, "SIGTERM")
+  // A gracefully stopping server may remove its registration before it releases its port, so only
+  // the exact process we signalled can tell us when it is safe for a restart to continue.
   if (!(await waitUntilStopped(info.pid, timing))) {
-    const latest = await read(options.file)
-    if (latest === undefined || !same(latest, info)) return
     signal(info.pid, "SIGKILL")
     if (!(await waitUntilStopped(info.pid, timing))) throw new Error(`Server process ${info.pid} is still running`)
   }

packages/client/test/fixture/service.ts

@@ -1,4 +1,4 @@
-import { appendFile, rename, writeFile } from "node:fs/promises"
+import { appendFile, rename, rm, writeFile } from "node:fs/promises"
 
 const [registration, mode, delay] = process.argv.slice(2)
 if (registration === undefined || mode === undefined) throw new Error("Missing service fixture arguments")
@@ -98,6 +98,11 @@ await rename(registration + ".tmp", registration)
 
 async function shutdown(signal?: NodeJS.Signals) {
   if (signal !== undefined) await writeFile(registration + ".signal", signal)
+  // A lingering server removes its registration first and keeps its port while it winds down.
+  if (mode === "lingering") {
+    await rm(registration, { force: true })
+    await Bun.sleep(Number(delay))
+  }
   server.stop(true)
   process.exit()
 }

packages/client/test/promise-service.test.ts

@@ -4,6 +4,7 @@ import { serviceFixture } from "./fixture/service-fixture"
 import { accelerate } from "./fixture/service-timing"
 
 const ensure = accelerate(Service.ensure)
+const stop = accelerate(Service.stop)
 
 test("discovers a registered service", async () => {
   await using fixture = await serviceFixture()
@@ -158,3 +159,20 @@ test("signals the registered service process", async () => {
   expect(await Bun.file(registration + ".signal").text()).toBe("SIGTERM")
   expect(await Bun.file(registration).exists()).toBe(false)
 })
+
+test("stop outlives a server that unregisters before releasing its port", async () => {
+  await using fixture = await serviceFixture()
+  const registration = fixture.registration
+  const existing = fixture.spawn("lingering", "5000")
+  await fixture.waitForFile()
+  const original = await Bun.file(registration).json()
+
+  await stop({ file: registration })
+
+  expect(await Bun.file(registration + ".signal").text()).toBe("SIGTERM")
+  expect(() => process.kill(original.pid, 0)).toThrow()
+  const listener = Bun.serve({ port: Number(new URL(original.url).port), fetch: () => new Response() })
+  await listener.stop(true)
+  expect(await Bun.file(registration).exists()).toBe(false)
+  await existing.exited
+}, 15_000)

packages/client/test/service.test.ts

@@ -7,6 +7,7 @@ import { serviceFixture } from "./fixture/service-fixture"
 import { accelerate } from "./fixture/service-timing"
 
 const ensure = accelerate(Service.ensure)
+const stop = accelerate(Service.stop)
 
 test("a concurrent same-version start cannot invalidate a resolved endpoint", async () => {
   await using fixture = await serviceFixture()
@@ -161,6 +162,22 @@ test("signals an unresponsive registered service process", async () => {
   expect(await Bun.file(registration).exists()).toBe(false)
 })
 
+test("stop outlives a server that unregisters before releasing its port", async () => {
+  await using fixture = await serviceFixture()
+  const registration = fixture.registration
+  const existing = fixture.spawn("lingering", "5000")
+  await fixture.waitForFile()
+  const original = await Bun.file(registration).json()
+
+  await run(stop({ file: registration }))
+
+  expect(await Bun.file(registration + ".signal").text()).toBe("SIGTERM")
+  expect(() => process.kill(original.pid, 0)).toThrow()
+  await expectPortAvailable(Number(new URL(original.url).port))
+  expect(await Bun.file(registration).exists()).toBe(false)
+  await existing.exited
+}, 15_000)
+
 test("signals an incompatible service before starting its replacement", async () => {
   await using fixture = await serviceFixture()
   const registration = fixture.registration
@@ -302,3 +319,8 @@ function run<A, E>(effect: Effect.Effect<A, E, FileSystem.FileSystem>) {
 async function status(url: string) {
   return fetch(new URL("/api/info", url), { signal: AbortSignal.timeout(1_000) }).then((response) => response.json())
 }
+
+async function expectPortAvailable(port: number) {
+  const server = Bun.serve({ port, fetch: () => new Response() })
+  await server.stop(true)
+}