(High Severity Concurrency Bug): Double Teardown (`t.teardown()`) Invocation on Test Timeout or Inactivity
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);
}
}
}finish()has no guard against being called multiple times (this.finishResultis not cached).this.teardownsis 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();Source: avajs/ava