Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion test/expectations.txt
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,6 @@ test/js/bun/spawn/spawn-maxbuf.test.ts [ FLAKY ]

# Tests failed due to memory leaks
[ ASAN ] test/js/node/url/pathToFileURL.test.ts [ LEAK ] # pathToFileURL doesn't leak memory
[ ASAN ] test/js/node/fs/abort-signal-leak-read-write-file.test.ts [ LEAK ] # should not leak memory with already aborted signals
[ ASAN ] test/js/web/streams/streams-leak.test.ts [ LEAK ] # Absolute memory usage remains relatively constant when reading and writing to a pipe
[ ASAN ] test/cli/run/require-cache.test.ts [ LEAK ] # files transpiled and loaded don't leak file paths > via require()

Expand Down
41 changes: 0 additions & 41 deletions test/js/node/fs/abort-signal-leak-read-write-file-fixture.ts

This file was deleted.

48 changes: 45 additions & 3 deletions test/js/node/fs/abort-signal-leak-read-write-file.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,48 @@
import { heapStats } from "bun:jsc";
import { expect, test } from "bun:test";
import path from "path";
import fs from "fs";
import { expectMaxObjectTypeCount, isASAN, isDebug, tempDir } from "harness";
import { join } from "path";

test("should not leak memory with already aborted signals", async () => {
expect([path.join(import.meta.dir, "abort-signal-leak-read-write-file-fixture.ts")]).toRun();
// https://github.com/oven-sh/bun/pull/16788
test("fs.promises readFile/writeFile does not leak AbortSignal", async () => {
Comment thread
robobun marked this conversation as resolved.
using dir = tempDir("fs-abort-signal-leak", { blah: "blah" });
const target = join(String(dir), "blah");
const noop = () => {};

// pre-aborted signal: readFile/writeFile reject immediately and must release
// the signal reference on the early-return path.
await expect(fs.promises.readFile(target, { signal: AbortSignal.abort() })).rejects.toMatchObject({
name: "AbortError",
});
await expect(fs.promises.writeFile(target, "blah", { signal: AbortSignal.abort() })).rejects.toMatchObject({
name: "AbortError",
});

// abort after the write has been scheduled: the async completion path must
// also release the signal reference. The write may legitimately finish before
// nextTick fires, so either resolution or AbortError is acceptable here.
{
const controller = new AbortController();
const prom = fs.promises.writeFile(target, "blah", { signal: controller.signal });
process.nextTick(() => controller.abort());
await prom.catch(e => expect(e).toMatchObject({ name: "AbortError" }));
}

// Leak detector. A JSAbortSignal wrapper is only pinned by hasPendingActivity()
// when the signal is NOT aborted AND has an abort listener (see
// JSAbortSignalCustom.cpp), so the loop below uses a never-aborted signal with
// a no-op listener: if readFile/writeFile forget to drop the pending-activity
// ref they take, every one of these wrappers survives GC and the object count
// below climbs with ITERATIONS instead of staying flat.
const baseline = heapStats().objectTypeCounts.AbortSignal ?? 0;
const ITERATIONS = isDebug || isASAN ? 100 : 2_000;
for (let i = 0; i < ITERATIONS; i++) {
const controller = new AbortController();
controller.signal.addEventListener("abort", noop);
await fs.promises.writeFile(target, "blah", { signal: controller.signal });
await fs.promises.readFile(target, { signal: controller.signal });
}

await expectMaxObjectTypeCount(expect, "AbortSignal", baseline + 10);
});
Loading