From ad6e14a2459c6870d844fc7c5cfba3c4e44ae594 Mon Sep 17 00:00:00 2001 From: Dan O'Leary Date: Sat, 8 Aug 2026 09:37:36 -0700 Subject: [PATCH 1/4] fix: cancel queued advertisement after service destroy --- src/CiaoService.ts | 19 ++++- src/Responder.ts | 17 ++++- src/advertise-destroy-race.spec.ts | 117 +++++++++++++++++++++++++++++ 3 files changed, 148 insertions(+), 5 deletions(-) create mode 100644 src/advertise-destroy-race.spec.ts diff --git a/src/CiaoService.ts b/src/CiaoService.ts index 45c7950..239fd00 100644 --- a/src/CiaoService.ts +++ b/src/CiaoService.ts @@ -449,12 +449,27 @@ export class CiaoService extends EventEmitter { * @returns */ public async destroy(): Promise { - await this.end(); - + const endPromise = this.end(); this.destroyed = true; + + try { + await endPromise; + } catch (error) { + this.destroyed = false; + throw error; + } + this.removeAllListeners(); } + /** + * Internal lifecycle query used by {@link Responder} to avoid directly reading private state. + * @private + */ + public isDestroyed(): boolean { + return this.destroyed; + } + /** * @returns The fully qualified domain name of the service, used to identify the service. */ diff --git a/src/Responder.ts b/src/Responder.ts index 3377718..751e8f2 100644 --- a/src/Responder.ts +++ b/src/Responder.ts @@ -283,11 +283,22 @@ export class Responder implements PacketHandler { // - Prober will call the Responder to generate responses to its queries to // resolve name conflicts the same way as with other services on the network - this.promiseChain = this.promiseChain // we synchronize all ongoing probes here - .then(() => service.rebuildServiceRecords()) // build the records the first time for the prober - .then(() => this.probe(service)); // probe errors are catch below + this.promiseChain = this.promiseChain.then(() => { + if (service.isDestroyed()) { + throw Prober.CANCEL_REASON; + } + + service.rebuildServiceRecords(); + return this.probe(service); // probe errors are catch below + }); return this.promiseChain.then(() => { + if (service.isDestroyed()) { + service.serviceState = ServiceState.UNANNOUNCED; + callback(); + return; + } + // we are not returning the promise returned by announced here, only PROBING is synchronized this.announce(service).catch(reason => { // handle announce errors diff --git a/src/advertise-destroy-race.spec.ts b/src/advertise-destroy-race.spec.ts new file mode 100644 index 0000000..6a8dc94 --- /dev/null +++ b/src/advertise-destroy-race.spec.ts @@ -0,0 +1,117 @@ +import { CiaoService, InternalServiceEvent, ServiceState } from "./CiaoService"; +import { Responder } from "./Responder"; +import { NetworkManager } from "./NetworkManager"; +import { Announcer } from "./responder/Announcer"; + +interface FakeResponderInternal { + server: { + sendQueryBroadcast: jest.Mock; + getBoundInterfaceNames: jest.Mock; + send: jest.Mock; + }; + promiseChain: Promise; + servicePointer: Map; + announcedServices: Map; + getAnnouncedServices: () => IterableIterator; +} + +interface FakeResponderFacade { + advertiseService: (service: CiaoService, callback: (error?: Error | undefined) => void) => Promise; + unpublishService: (service: CiaoService) => Promise; +} + +function makeFakeResponder() { + const server = { + sendQueryBroadcast: jest.fn(async () => []), + getBoundInterfaceNames: jest.fn(() => []), + send: jest.fn(async () => ({ status: "fulfilled", interface: "en0" })), + }; + + const responder = Object.create(Responder.prototype) as unknown as FakeResponderInternal; + responder.server = server; + responder.promiseChain = Promise.resolve(); + responder.servicePointer = new Map(); + responder.announcedServices = new Map(); + responder.getAnnouncedServices = () => responder.announcedServices.values(); + + return { responder, server }; +} + +function makeService(): CiaoService { + const networkManager = { getInterfaceMap: () => new Map() } as unknown as NetworkManager; + return new CiaoService(networkManager, { + name: "Test Service", + type: "http", + port: 4711, + }); +} + +function wire(responder: Responder | FakeResponderInternal, service: CiaoService): void { + const responderFacade = responder as unknown as FakeResponderFacade; + service.on(InternalServiceEvent.PUBLISH, responderFacade.advertiseService.bind(responderFacade, service)); + service.on(InternalServiceEvent.UNPUBLISH, responderFacade.unpublishService.bind(responderFacade, service)); +} + +async function flush(): Promise { + await Promise.resolve(); + await Promise.resolve(); +} + +async function advance(ms: number): Promise { + jest.advanceTimersByTime(ms); + await flush(); +} + +describe("immediate advertise()/destroy() lifecycle race", () => { + let announceSpy: jest.SpiedFunction; + let randomSpy: jest.SpiedFunction; + + beforeEach(() => { + jest.useFakeTimers(); + announceSpy = jest.spyOn(Announcer.prototype, "announce"); + randomSpy = jest.spyOn(Math, "random").mockReturnValue(0); + }); + + afterEach(() => { + jest.useRealTimers(); + announceSpy.mockRestore(); + randomSpy.mockRestore(); + }); + + it("does not probe, announce, or retry after a destroy raced before startup", async () => { + const { responder, server } = makeFakeResponder(); + const service = makeService(); + wire(responder, service); + + const advertisePromise = service.advertise(); + expect(service.serviceState).toBe(ServiceState.UNANNOUNCED); + + await service.destroy(); + + for (let i = 0; i < 20; i++) { + await advance(250); + } + + expect({ + probeBroadcasts: server.sendQueryBroadcast.mock.calls.length, + announcements: announceSpy.mock.calls.length, + serviceState: service.serviceState, + announcedServices: Array.from(responder.getAnnouncedServices()).length, + }).toEqual({ + probeBroadcasts: 0, + announcements: 0, + serviceState: ServiceState.UNANNOUNCED, + announcedServices: 0, + }); + await expect(advertisePromise).resolves.toBeUndefined(); + expect(jest.getTimerCount()).toBe(0); + }); + + it("preserves the existing guard against destroying a service twice", async () => { + const service = makeService(); + + await service.destroy(); + + await expect(service.destroy()).rejects.toThrow("Cannot end destroyed service!"); + }); +}); From f44309a31d03df64273424eff05663bbe3d3a73c Mon Sep 17 00:00:00 2001 From: Dan O'Leary Date: Sat, 8 Aug 2026 09:42:22 -0700 Subject: [PATCH 2/4] test: harden advertise destroy race coverage --- src/advertise-destroy-race.spec.ts | 71 ++++++++++++++++++++++++++++++ 1 file changed, 71 insertions(+) diff --git a/src/advertise-destroy-race.spec.ts b/src/advertise-destroy-race.spec.ts index 6a8dc94..57928c2 100644 --- a/src/advertise-destroy-race.spec.ts +++ b/src/advertise-destroy-race.spec.ts @@ -13,6 +13,7 @@ interface FakeResponderInternal { servicePointer: Map; announcedServices: Map; getAnnouncedServices: () => IterableIterator; + probe?: (service: CiaoService) => Promise; } interface FakeResponderFacade { @@ -107,6 +108,76 @@ describe("immediate advertise()/destroy() lifecycle race", () => { expect(jest.getTimerCount()).toBe(0); }); + it("does not announce when destroy lands after probing has started", async () => { + const { responder } = makeFakeResponder(); + const service = makeService(); + wire(responder, service); + + let finishProbe: () => void = () => { + throw new Error("probe did not start"); + }; + const probePromise = new Promise(resolve => { + finishProbe = resolve; + }); + const probe = jest.fn((probingService: CiaoService) => { + probingService.serviceState = ServiceState.PROBING; + return probePromise.then(() => { + probingService.serviceState = ServiceState.PROBED; + }); + }); + responder.probe = probe; + + const advertisePromise = service.advertise(); + await flush(); + + expect(probe).toHaveBeenCalledTimes(1); + expect(service.serviceState).toBe(ServiceState.PROBING); + + await service.destroy(); + finishProbe(); + await flush(); + + await expect(advertisePromise).resolves.toBeUndefined(); + expect(announceSpy).not.toHaveBeenCalled(); + expect(service.serviceState).toBe(ServiceState.UNANNOUNCED); + expect(Array.from(responder.getAnnouncedServices())).toHaveLength(0); + expect(jest.getTimerCount()).toBe(0); + }); + + it("survives repeated immediate advertise/destroy races without leftover work", async () => { + const cycles = 50; + + for (let i = 0; i < cycles; i++) { + const { responder, server } = makeFakeResponder(); + const service = makeService(); + wire(responder, service); + + const advertisePromise = service.advertise(); + await service.destroy(); + await expect(advertisePromise).resolves.toBeUndefined(); + + expect(server.sendQueryBroadcast).not.toHaveBeenCalled(); + expect(service.serviceState).toBe(ServiceState.UNANNOUNCED); + expect(Array.from(responder.getAnnouncedServices())).toHaveLength(0); + } + + await advance(5000); + expect(announceSpy).not.toHaveBeenCalled(); + expect(jest.getTimerCount()).toBe(0); + }); + + it("keeps the service usable when shutdown rejects", async () => { + const service = makeService(); + const shutdownError = new Error("goodbye failed"); + service.serviceState = ServiceState.ANNOUNCED; + service.on(InternalServiceEvent.UNPUBLISH, callback => callback(shutdownError)); + + await expect(service.destroy()).rejects.toThrow(shutdownError); + + expect(service.isDestroyed()).toBe(false); + expect(service.listenerCount(InternalServiceEvent.UNPUBLISH)).toBe(1); + }); + it("preserves the existing guard against destroying a service twice", async () => { const service = makeService(); From 6cd0ac042ec4ef17dde791f83d660e82374c3a7e Mon Sep 17 00:00:00 2001 From: Dan O'Leary Date: Sat, 8 Aug 2026 09:42:56 -0700 Subject: [PATCH 3/4] test: keep lifecycle regression compatible with old code --- src/advertise-destroy-race.spec.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/advertise-destroy-race.spec.ts b/src/advertise-destroy-race.spec.ts index 57928c2..176aa6e 100644 --- a/src/advertise-destroy-race.spec.ts +++ b/src/advertise-destroy-race.spec.ts @@ -169,12 +169,16 @@ describe("immediate advertise()/destroy() lifecycle race", () => { it("keeps the service usable when shutdown rejects", async () => { const service = makeService(); const shutdownError = new Error("goodbye failed"); + let shutdownAttempts = 0; service.serviceState = ServiceState.ANNOUNCED; - service.on(InternalServiceEvent.UNPUBLISH, callback => callback(shutdownError)); + service.on(InternalServiceEvent.UNPUBLISH, callback => { + callback(shutdownAttempts++ === 0? shutdownError: undefined); + }); await expect(service.destroy()).rejects.toThrow(shutdownError); - expect(service.isDestroyed()).toBe(false); + await expect(service.end()).resolves.toBeUndefined(); + expect(shutdownAttempts).toBe(2); expect(service.listenerCount(InternalServiceEvent.UNPUBLISH)).toBe(1); }); From 17320bc7bfd9a4c95a27a7063a630bdbe14b305f Mon Sep 17 00:00:00 2001 From: Dan O'Leary Date: Sat, 8 Aug 2026 10:37:35 -0700 Subject: [PATCH 4/4] docs: add pending release note --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a30736c..2a92069 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ All notable changes to `@homebridge/ciao` will be documented in this file. This - fix: send the probe and announce retry chatter to debug instead of the console (#72) - fix: let an empty DEBUG decline a prerelease build's automatic debug output (#72) - fix: read DEBUG before the debug package erases an empty one (#72) +- fix: cancel queued advertisement after service destruction (#73) ## v1.3.10 (2026-07-08)