(High Severity Concurrency Bug): Double Teardown (t.teardown()) Invocation on Test Timeout or Inactivity #3482

Issue :- (High Severity Concurrency Bug): Double Teardown (t.teardown()) Invocation on Test Timeout or Inactivity

Metadata

• Title: [Bug] Test timeouts and inactivity cause t.teardown() hooks to be executed twice
• Severity: High (Flaky Tests / Double-Free & Teardown State Corruption)
• Component: Test Execution Engine (lib/test.js)
• Affected File: lib/test.js (run, finish, timeout, runTeardowns)

Problem Summary

In lib/test.js, when a test returns a Promise/Observable and encounters a timeout (this.finishDueToTimeout()) or inactivity (this.finishDueToInactivity()), resolve(this.finish()) is immediately invoked. This executes this.finish(), running all teardowns registered via t.teardown().

However, the test's user-provided asynchronous function continues running in the background. When that promise eventually settles (resolves or rejects) at a later time, its .then(() => resolve(this.finish())) handler executes this.finish() a second time.

Because finish() does not check if it has already completed, and this.teardowns is not cleared after execution, this.runTeardowns() iterates over all teardowns again. Any non-idempotent teardown hook (e.g. server.close(), db.disconnect(), removing temp directories, or closing file handles) throws double-free errors, causing unhandled rejections or crashes.

Root Cause Analysis

Look at lib/test.js lines 581–619:

if (promise) {
    return new Promise(resolve => {
        this.finishDueToAttributedError = () => {
            resolve(this.finish());
        };

        this.finishDueToTimeout = () => {
            resolve(this.finish()); // <--- 1st invocation of this.finish()
        };

        this.finishDueToInactivity = () => {
            const error = returnedObservable
                ? new Error('Observable returned by test never completed')
                : new Error('Promise returned by test never resolved');
            this.saveFirstError(error);
            resolve(this.finish()); // <--- 1st invocation of this.finish()
        };

        promise
            .catch(error => { ... })
            .then(() => resolve(this.finish())); // <--- 2nd invocation when promise settles!
    });
}

Now trace finish() lines 624–635:

async finish() {
    this.finishing = true;

    this.clearTimeout();
    this.verifyPlan();
    this.verifyAssertions();
    await this.runTeardowns(); // <--- Runs teardowns!
    ...

And runTeardowns() lines 480–490:

async runTeardowns() {
    const teardowns = this.teardowns.toReversed();

    for (const teardown of teardowns) {
        try {
            await teardown();
        } catch (error) {
            this.saveFirstError(error);
        }
    }
}

1. finish() has no guard against being called multiple times (this.finishResult is not cached).
2. this.teardowns is never cleared (this.teardowns = []), allowing the second invocation to re-execute every teardown function.

Proposed Fix

Cache the finish promise so that finish() is strictly idempotent, and drain the teardowns array upon execution:

--- a/lib/test.js
+++ b/lib/test.js
@@ -294,6 +294,7 @@ export default class Test {
 		this.finishDueToInactivity = null;
 		this.finishDueToTimeout = null;
 		this.finishing = false;
+		this.finishPromise = null;
 		this.pendingAssertionCount = 0;
 		this.pendingAssertionMetadata = new Set();
 		this.pendingAttemptCount = 0;
@@ -479,7 +480,8 @@ export default class Test {
 	}
 
 	async runTeardowns() {
-		const teardowns = this.teardowns.toReversed();
+		const teardowns = this.teardowns.splice(0).toReversed();
 
 		for (const teardown of teardowns) {
 			try {
@@ -622,6 +624,14 @@ export default class Test {
 	}
 
 	async finish() {
+		if (this.finishPromise) {
+			return this.finishPromise;
+		}
+
+		return (this.finishPromise = this._finishInternal());
+	}
+
+	async _finishInternal() {
 		this.finishing = true;
 
 		this.clearTimeout();