From f1992f79c2f0b5b2febc95a9e5082a0e8e8f3f80 Mon Sep 17 00:00:00 2001 From: Hector Hernandez <39923391+hectorhdzg@users.noreply.github.com> Date: Fri, 29 May 2026 15:24:33 -0700 Subject: [PATCH 1/2] fix: clear polling timer on LiveMetrics shutdown to prevent resource leak LiveMetrics.shutdown() only called meterProvider.shutdown() but never cleared the active setTimeout handle. This caused the QuickPulse polling loop to continue indefinitely after shutdown, leading to: - Resource leaks in long-running processes that reinitialize the SDK - Stale QuickPulse traffic sent after the SDK is torn down - Process hang on exit (active timer ref keeps the event loop alive) - Amplified impact in serverless/container environments with frequent recycling Changes: - Add _isShutdown flag to prevent timer rescheduling after shutdown - Call clearTimeout(this.handle) in shutdown() to cancel the active timer - Call deactivateMetrics() in shutdown() for full cleanup - Guard goQuickpulse() and quickPulseDone() with early return on _isShutdown - Add 6 unit tests covering timer clearing, flag, deactivation, no-reschedule, and idempotency --- .../metrics/quickpulse/liveMetrics.ts | 11 +++- .../internal/unit/metrics/liveMetrics.test.ts | 50 +++++++++++++++++++ 2 files changed, 60 insertions(+), 1 deletion(-) diff --git a/src/azureMonitor/metrics/quickpulse/liveMetrics.ts b/src/azureMonitor/metrics/quickpulse/liveMetrics.ts index ee5bf77..47ef465 100644 --- a/src/azureMonitor/metrics/quickpulse/liveMetrics.ts +++ b/src/azureMonitor/metrics/quickpulse/liveMetrics.ts @@ -137,6 +137,7 @@ export class LiveMetrics { private derivedMetricProjection: Projection = new Projection(); private validator: Validator = new Validator(); private filter: Filter = new Filter(); + private _isShutdown: boolean = false; // type: Map> private validDocumentFilterConjuctionGroupInfos: Map< string, @@ -203,10 +204,15 @@ export class LiveMetrics { } public shutdown(): void { - this.meterProvider?.shutdown(); + this._isShutdown = true; + clearTimeout(this.handle as any); + this.deactivateMetrics(); } private async goQuickpulse(): Promise { + if (this._isShutdown) { + return; + } if (!this.isCollectingData) { // If not collecting, Ping try { @@ -232,6 +238,9 @@ export class LiveMetrics { } private async quickPulseDone(response: QuickpulseResponse | undefined): Promise { + if (this._isShutdown) { + return; + } if (!response) { if (!this.isCollectingData) { if (Date.now() - this.lastSuccessTime >= MAX_PING_WAIT_TIME) { diff --git a/test/internal/unit/metrics/liveMetrics.test.ts b/test/internal/unit/metrics/liveMetrics.test.ts index 44857c2..36f2efb 100644 --- a/test/internal/unit/metrics/liveMetrics.test.ts +++ b/test/internal/unit/metrics/liveMetrics.test.ts @@ -671,4 +671,54 @@ describe("#LiveMetrics", () => { ["testScope1"], ); }); + + it("shutdown should clear the polling timer", () => { + const clearTimeoutSpy = vi.spyOn(global, "clearTimeout"); + const handle = autoCollect["handle"]; + autoCollect.shutdown(); + expect(clearTimeoutSpy).toHaveBeenCalledWith(handle); + clearTimeoutSpy.mockRestore(); + }); + + it("shutdown should set _isShutdown flag", () => { + expect(autoCollect["_isShutdown"]).toBe(false); + autoCollect.shutdown(); + expect(autoCollect["_isShutdown"]).toBe(true); + }); + + it("shutdown should deactivate metrics", () => { + autoCollect.activateMetrics({ collectionInterval: 100 }); + expect(autoCollect["meterProvider"]).toBeDefined(); + autoCollect.shutdown(); + expect(autoCollect["meterProvider"]).toBeUndefined(); + }); + + it("goQuickpulse should not reschedule after shutdown", async () => { + autoCollect.shutdown(); + const setTimeoutSpy = vi.spyOn(global, "setTimeout"); + await autoCollect["goQuickpulse"](); + // goQuickpulse should early-return without scheduling a new timer + expect(setTimeoutSpy).not.toHaveBeenCalled(); + setTimeoutSpy.mockRestore(); + }); + + it("quickPulseDone should not reschedule after shutdown", async () => { + autoCollect.shutdown(); + const setTimeoutSpy = vi.spyOn(global, "setTimeout"); + await autoCollect["quickPulseDone"]({ + xMsQpsSubscribed: "false", + xMsQpsConfigurationEtag: undefined, + } as any); + expect(setTimeoutSpy).not.toHaveBeenCalled(); + setTimeoutSpy.mockRestore(); + }); + + it("shutdown should be idempotent", () => { + autoCollect.activateMetrics({ collectionInterval: 100 }); + autoCollect.shutdown(); + // Second shutdown should not throw + expect(() => autoCollect.shutdown()).not.toThrow(); + expect(autoCollect["_isShutdown"]).toBe(true); + expect(autoCollect["meterProvider"]).toBeUndefined(); + }); }); From a207d8bb3a8b5b970fe7d35b818d72e2734662dc Mon Sep 17 00:00:00 2001 From: Hector Hernandez <39923391+hectorhdzg@users.noreply.github.com> Date: Fri, 29 May 2026 15:37:08 -0700 Subject: [PATCH 2/2] fix: guard setTimeout after await to prevent race with concurrent shutdown Address review feedback: shutdown() called during an in-flight ping request could allow goQuickpulse() to resume and reschedule the timer after clearTimeout() already ran. Added _isShutdown checks immediately before each setTimeout call in goQuickpulse() and quickPulseDone(), plus before activateMetrics(). Added test covering the mid-ping shutdown race condition. --- .../metrics/quickpulse/liveMetrics.ts | 14 +++++++++----- test/internal/unit/metrics/liveMetrics.test.ts | 15 +++++++++++++++ 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/src/azureMonitor/metrics/quickpulse/liveMetrics.ts b/src/azureMonitor/metrics/quickpulse/liveMetrics.ts index 47ef465..5428b70 100644 --- a/src/azureMonitor/metrics/quickpulse/liveMetrics.ts +++ b/src/azureMonitor/metrics/quickpulse/liveMetrics.ts @@ -229,10 +229,12 @@ export class LiveMetrics { this.quickPulseDone(undefined); } - this.handle = setTimeout(this.goQuickpulse.bind(this), this.pingInterval); - this.handle.unref(); + if (!this._isShutdown) { + this.handle = setTimeout(this.goQuickpulse.bind(this), this.pingInterval); + this.handle.unref(); + } } - if (this.isCollectingData) { + if (this.isCollectingData && !this._isShutdown) { this.activateMetrics({ collectionInterval: this.postInterval }); } } @@ -268,8 +270,10 @@ export class LiveMetrics { this.etag = ""; this.deactivateMetrics(); - this.handle = setTimeout(this.goQuickpulse.bind(this), this.pingInterval); - this.handle.unref(); + if (!this._isShutdown) { + this.handle = setTimeout(this.goQuickpulse.bind(this), this.pingInterval); + this.handle.unref(); + } } const endpointRedirect = response.xMsQpsServiceEndpointRedirectV2; diff --git a/test/internal/unit/metrics/liveMetrics.test.ts b/test/internal/unit/metrics/liveMetrics.test.ts index 36f2efb..e203393 100644 --- a/test/internal/unit/metrics/liveMetrics.test.ts +++ b/test/internal/unit/metrics/liveMetrics.test.ts @@ -702,6 +702,21 @@ describe("#LiveMetrics", () => { setTimeoutSpy.mockRestore(); }); + it("goQuickpulse should not reschedule if shutdown called during ping", async () => { + // Simulate shutdown happening while the ping request is in-flight + vi.spyOn(autoCollect["pingSender"], "isSubscribed").mockImplementation(async () => { + // Shutdown is called while we are awaiting the ping + autoCollect.shutdown(); + return undefined as any; + }); + autoCollect["_isShutdown"] = false; // reset so goQuickpulse enters the body + autoCollect["isCollectingData"] = false; + const setTimeoutSpy = vi.spyOn(global, "setTimeout"); + await autoCollect["goQuickpulse"](); + expect(setTimeoutSpy).not.toHaveBeenCalled(); + setTimeoutSpy.mockRestore(); + }); + it("quickPulseDone should not reschedule after shutdown", async () => { autoCollect.shutdown(); const setTimeoutSpy = vi.spyOn(global, "setTimeout");