fix(util-waiter): add event listener cleanup (#1633)

Author: kuhe

.changeset/gentle-hornets-relax.md

@@ -0,0 +1,5 @@
+---
+"@smithy/util-waiter": patch
+---
+
+clean up waiters' abort signal listener

packages/util-waiter/src/createWaiter.spec.ts

@@ -1,14 +1,12 @@
-import { AbortController } from "@smithy/abort-controller";
 import { afterEach, beforeEach, describe, expect, test as it, vi } from "vitest";
 
+import { createWaiter } from "./createWaiter";
 import { WaiterOptions, WaiterState } from "./waiter";
 
 vi.mock("./utils/validate", () => ({
   validateWaiterOptions: vi.fn(),
 }));
 
-import { createWaiter } from "./createWaiter";
-
 describe("createWaiter", () => {
   beforeEach(() => {
     vi.useFakeTimers();
@@ -56,7 +54,28 @@ describe("createWaiter", () => {
     expect(await statusPromise).toMatchObject(abortedState);
   });
 
-  it("should success when acceptor checker returns seccess", async () => {
+  it("should remove the event listener on the abort signal after the waiter resolves regardless of whether it has been invoked", async () => {
+    const abortController = new AbortController();
+    vi.spyOn(abortController.signal, "addEventListener");
+    vi.spyOn(abortController.signal, "removeEventListener");
+
+    const mockAcceptorChecks = vi.fn().mockResolvedValue(successState);
+    const statusPromise = createWaiter(
+      {
+        ...minimalWaiterConfig,
+        abortSignal: abortController.signal,
+        maxWaitTime: 20,
+      },
+      input,
+      mockAcceptorChecks
+    );
+    expect(abortController.signal.addEventListener).toHaveBeenCalledOnce();
+    vi.advanceTimersByTime(minimalWaiterConfig.minDelay * 1000);
+    expect(await statusPromise).toMatchObject(successState);
+    expect(abortController.signal.removeEventListener).toHaveBeenCalledOnce();
+  });
+
+  it("should succeed when acceptor checker returns success", async () => {
     const mockAcceptorChecks = vi.fn().mockResolvedValue(successState);
     const statusPromise = createWaiter(
       {

packages/util-waiter/src/createWaiter.ts

@@ -4,9 +4,16 @@ import { runPolling } from "./poller";
 import { validateWaiterOptions } from "./utils";
 import { WaiterOptions, WaiterResult, waiterServiceDefaults, WaiterState } from "./waiter";
 
-const abortTimeout = async (abortSignal: AbortSignal | DeprecatedAbortSignal): Promise<WaiterResult> => {
-  return new Promise((resolve) => {
-    const onAbort = () => resolve({ state: WaiterState.ABORTED });
+const abortTimeout = (
+  abortSignal: AbortSignal | DeprecatedAbortSignal
+): {
+  clearListener: () => void;
+  aborted: Promise<WaiterResult>;
+} => {
+  let onAbort: () => void;
+
+  const promise = new Promise<WaiterResult>((resolve) => {
+    onAbort = () => resolve({ state: WaiterState.ABORTED });
     if (typeof (abortSignal as AbortSignal).addEventListener === "function") {
       // preferred.
       (abortSignal as AbortSignal).addEventListener("abort", onAbort);
@@ -15,6 +22,15 @@ const abortTimeout = async (abortSignal: AbortSignal | DeprecatedAbortSignal): P
       abortSignal.onabort = onAbort;
     }
   });
+
+  return {
+    clearListener() {
+      if (typeof (abortSignal as AbortSignal).removeEventListener === "function") {
+        (abortSignal as AbortSignal).removeEventListener("abort", onAbort);
+      }
+    },
+    aborted: promise,
+  };
 };
 
 /**
@@ -38,13 +54,24 @@ export const createWaiter = async <Client, Input>(
   validateWaiterOptions(params);
 
   const exitConditions = [runPolling<Client, Input>(params, input, acceptorChecks)];
-  if (options.abortController) {
-    exitConditions.push(abortTimeout(options.abortController.signal));
-  }
+
+  const finalize = [] as Array<() => void>;
 
   if (options.abortSignal) {
-    exitConditions.push(abortTimeout(options.abortSignal));
+    const { aborted, clearListener } = abortTimeout(options.abortSignal);
+    finalize.push(clearListener);
+    exitConditions.push(aborted);
+  }
+  if (options.abortController?.signal) {
+    const { aborted, clearListener } = abortTimeout(options.abortController.signal);
+    finalize.push(clearListener);
+    exitConditions.push(aborted);
   }
 
-  return Promise.race(exitConditions);
+  return Promise.race<WaiterResult>(exitConditions).then((result) => {
+    for (const fn of finalize) {
+      fn();
+    }
+    return result;
+  });
 };