From 0baf223b1beb38b54bfd3f143b2e67ccc0595840 Mon Sep 17 00:00:00 2001 From: Cristian Greco Date: Mon, 12 Oct 2026 18:58:07 +0100 Subject: [PATCH 1/2] Fix container leak when start fails after the container is created A failure between container creation and start() returning, such as a throwing containerCreated, containerStarting or containerStarted hook, rejected start() but left the container behind with no handle to stop it. startContainer now removes the container before rethrowing, as waitForContainer already does for a failed wait strategy. --- .../generic-container-lifecycle.test.ts | 47 ++++++++++++++++++- .../generic-container/generic-container.ts | 21 +++++++++ 2 files changed, 67 insertions(+), 1 deletion(-) diff --git a/packages/testcontainers/src/generic-container/generic-container-lifecycle.test.ts b/packages/testcontainers/src/generic-container/generic-container-lifecycle.test.ts index 826eb1a86..6f91f57e1 100644 --- a/packages/testcontainers/src/generic-container/generic-container-lifecycle.test.ts +++ b/packages/testcontainers/src/generic-container/generic-container-lifecycle.test.ts @@ -1,5 +1,12 @@ import type { Mock } from "vitest"; -import { AbstractStartedContainer, GenericContainer, type InspectResult, type StartedTestContainer } from "../index"; +import { RandomUuid } from "../common"; +import { + AbstractStartedContainer, + GenericContainer, + getContainerRuntimeClient, + type InspectResult, + type StartedTestContainer, +} from "../index"; describe("GenericContainer lifecycle", { concurrent: false, timeout: 180_000 }, () => { let beforeContainerCreated: Mock; @@ -50,6 +57,44 @@ describe("GenericContainer lifecycle", { concurrent: false, timeout: 180_000 }, expect(containerStarted).not.toHaveBeenCalled(); }); + it.each(["containerCreated", "containerStarting", "containerStarted"] as const)( + "should remove the container when the %s callback fails", + async (callback) => { + const containerName = `container-${new RandomUuid().nextUuid()}`; + ({ containerCreated, containerStarting, containerStarted })[callback].mockImplementation(() => { + throw new Error(`${callback} failed`); + }); + + await expect( + new CustomContainer("cristianrgreco/testcontainer:1.1.14") + .withName(containerName) + .withExposedPorts(8080) + .start() + ).rejects.toThrowError(`${callback} failed`); + + const client = await getContainerRuntimeClient(); + await expect(client.container.getById(containerName).inspect()).rejects.toThrow(/404/); + } + ); + + it("should not reuse a container when a lifecycle callback failed", async () => { + const labels = { "failed-start": new RandomUuid().nextUuid() }; + const startContainer = () => + new CustomContainer("cristianrgreco/testcontainer:1.1.14") + .withLabels(labels) + .withExposedPorts(8080) + .withReuse() + .start(); + containerStarted.mockImplementationOnce(() => { + throw new Error("containerStarted failed"); + }); + + await expect(startContainer()).rejects.toThrowError("containerStarted failed"); + await using container = await startContainer(); + + expect(container.getId()).not.toEqual(containerCreated.mock.calls[0][0]); + }); + class CustomContainer extends GenericContainer { protected override async beforeContainerCreated(): Promise { beforeContainerCreated(); diff --git a/packages/testcontainers/src/generic-container/generic-container.ts b/packages/testcontainers/src/generic-container/generic-container.ts index d9b2f8ffd..952e7cb64 100644 --- a/packages/testcontainers/src/generic-container/generic-container.ts +++ b/packages/testcontainers/src/generic-container/generic-container.ts @@ -199,6 +199,27 @@ export class GenericContainer implements TestContainer { private async startContainer(client: ContainerRuntimeClient): Promise { const container = await client.container.create({ ...this.createOpts, HostConfig: this.hostConfig }); + try { + return await this.startCreatedContainer(client, container); + } catch (err) { + try { + await container.remove({ force: true, v: true }); + } catch (removeErr) { + const alreadyRemoved = (removeErr as { statusCode?: number }).statusCode === 404; + if (!alreadyRemoved) { + log.error(`Failed to remove container after it failed to start: ${removeErr}`, { + containerId: container.id, + }); + } + } + throw err; + } + } + + private async startCreatedContainer( + client: ContainerRuntimeClient, + container: Container + ): Promise { if (!this.isHelperContainer() && PortForwarderInstance.isRunning()) { await this.connectContainerToPortForwarder(client, container); } From 894c02b8f6f135f4b8f740140fca90075e05118a Mon Sep 17 00:00:00 2001 From: Cristian Greco Date: Tue, 13 Oct 2026 10:45:48 +0100 Subject: [PATCH 2/2] Share the failed-container cleanup between start and waitForContainer startContainer and waitForContainer each removed the container in their own way. Both now call removeFailedContainer, which force-removes the container with its volumes and treats an already removed container as done. waitForContainer keeps its client parameter because it is exported. --- .../utils/remove-failed-container.ts | 16 ++++++++++++++++ .../src/generic-container/generic-container.ts | 12 ++---------- .../src/wait-strategies/wait-for-container.ts | 10 +++------- 3 files changed, 21 insertions(+), 17 deletions(-) create mode 100644 packages/testcontainers/src/container-runtime/utils/remove-failed-container.ts diff --git a/packages/testcontainers/src/container-runtime/utils/remove-failed-container.ts b/packages/testcontainers/src/container-runtime/utils/remove-failed-container.ts new file mode 100644 index 000000000..bdd8f0a6a --- /dev/null +++ b/packages/testcontainers/src/container-runtime/utils/remove-failed-container.ts @@ -0,0 +1,16 @@ +import type Dockerode from "dockerode"; +import { log } from "../../common"; + +export const removeFailedContainer = async (container: Dockerode.Container): Promise => { + try { + log.debug(`Removing container...`, { containerId: container.id }); + await container.remove({ force: true, v: true }); + log.debug(`Removed container`, { containerId: container.id }); + } catch (err) { + if ((err as { statusCode?: number }).statusCode === 404) { + log.debug(`Container already removed`, { containerId: container.id }); + } else { + log.error(`Failed to remove container: ${err}`, { containerId: container.id }); + } + } +}; diff --git a/packages/testcontainers/src/generic-container/generic-container.ts b/packages/testcontainers/src/generic-container/generic-container.ts index 952e7cb64..dc6778531 100644 --- a/packages/testcontainers/src/generic-container/generic-container.ts +++ b/packages/testcontainers/src/generic-container/generic-container.ts @@ -6,6 +6,7 @@ import type { Container, ContainerCreateOptions, ContainerInspectInfo, HostConfi import { containerLog, hash, log, toNanos } from "../common"; import { type ContainerRuntimeClient, getContainerRuntimeClient, ImageName } from "../container-runtime"; import { CONTAINER_STATUSES } from "../container-runtime/clients/container/types"; +import { removeFailedContainer } from "../container-runtime/utils/remove-failed-container"; import type { StartedNetwork } from "../network/network"; // biome-ignore lint/suspicious/noImportCycles: Ryuk and sshd run as GenericContainers; only dereferenced at runtime import { PortForwarderInstance, SSHD_IMAGE } from "../port-forwarder/port-forwarder"; @@ -202,16 +203,7 @@ export class GenericContainer implements TestContainer { try { return await this.startCreatedContainer(client, container); } catch (err) { - try { - await container.remove({ force: true, v: true }); - } catch (removeErr) { - const alreadyRemoved = (removeErr as { statusCode?: number }).statusCode === 404; - if (!alreadyRemoved) { - log.error(`Failed to remove container after it failed to start: ${removeErr}`, { - containerId: container.id, - }); - } - } + await removeFailedContainer(container); throw err; } } diff --git a/packages/testcontainers/src/wait-strategies/wait-for-container.ts b/packages/testcontainers/src/wait-strategies/wait-for-container.ts index ca8a3f190..cc118968b 100644 --- a/packages/testcontainers/src/wait-strategies/wait-for-container.ts +++ b/packages/testcontainers/src/wait-strategies/wait-for-container.ts @@ -1,11 +1,12 @@ import type { Container } from "dockerode"; import { log } from "../common"; import type { ContainerRuntimeClient } from "../container-runtime"; +import { removeFailedContainer } from "../container-runtime/utils/remove-failed-container"; import type { BoundPorts } from "../utils/bound-ports"; import type { WaitStrategy } from "./wait-strategy"; export const waitForContainer = async ( - client: ContainerRuntimeClient, + _client: ContainerRuntimeClient, container: Container, waitStrategy: WaitStrategy, boundPorts: BoundPorts, @@ -18,12 +19,7 @@ export const waitForContainer = async ( log.info(`Container is ready`, { containerId: container.id }); } catch (err) { log.error(`Container failed to be ready: ${err}`, { containerId: container.id }); - try { - await client.container.stop(container, { timeout: 0 }); - await client.container.remove(container, { removeVolumes: true }); - } catch (stopErr) { - log.error(`Failed to stop container after it failed to be ready: ${stopErr}`, { containerId: container.id }); - } + await removeFailedContainer(container); throw err; } };