Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
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
23 changes: 18 additions & 5 deletions src/jsc/VirtualMachine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3745,26 +3745,39 @@ impl VirtualMachine {
};
}
// Const-generic bool can't be `!ADD_DOUBLE_REF`, so branch.
let mut was_new = false;
let source = if ADD_DOUBLE_REF {
self.ref_counted_string::<false>(code, hash_)
self.ref_counted_string_with_was_new::<false>(&mut was_new, code, hash_)
} else {
self.ref_counted_string::<true>(code, hash_)
self.ref_counted_string_with_was_new::<true>(&mut was_new, code, hash_)
};
// SAFETY: `ref_counted_string` returns a live `*mut RefString` held in
// `self.ref_strings`; we own +1 (or +3 below) until JSC calls the
// SAFETY: `ref_counted_string_with_was_new` returns a live `*mut RefString`
// held in `self.ref_strings`; we own +1 (or +3 below) until JSC calls the
// external-string finalizer.
let source_ref = unsafe { &*source };
if ADD_DOUBLE_REF {
source_ref.ref_();
source_ref.ref_();
}

// `ref_strings` is a weak cache: entries self-remove via the external
// string finalizer (free_ref_string -> RefString::destroy ->
// clear_ref_string) when the impl refcount hits zero. Hand the caller
// exactly +1 that it will balance via `source_code_needs_deref = true`.
// A fresh entry already carries the +1 from `create_external`; a cache
// hit does not, so take one here. Without this the create_external +1
// was never released and every distinct transpiled source under --hot
// leaked its duped bytes and map slot forever.
Comment thread
robobun marked this conversation as resolved.
Outdated
if !was_new {
source_ref.ref_();
}
Comment thread
claude[bot] marked this conversation as resolved.
Outdated
Comment thread
claude[bot] marked this conversation as resolved.
Outdated

ResolvedSource {
source_code: bun_core::String::adopt_wtf_impl(source_ref.impl_),
specifier,
source_url: create_if_different(&specifier, source_url),
allocator: source.cast::<c_void>(),
source_code_needs_deref: false,
source_code_needs_deref: true,
..Default::default()
}
}
Expand Down
10 changes: 10 additions & 0 deletions src/jsc/bindings/ZigGlobalObject.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,7 @@
#include "EventLoopTask.h"
#include "NodeModuleModule.h"
#include <JavaScriptCore/JSCBytecodeCacheVersion.h>
#include <JavaScriptCore/CodeCache.h>
#include "JSPerformanceServerTiming.h"
#include "JSPerformanceResourceTiming.h"
#include "JSPerformanceTiming.h"
Expand Down Expand Up @@ -3536,6 +3537,15 @@ void GlobalObject::reload()
this->requireMap()->clear(this);
RETURN_IF_EXCEPTION(scope, );

// The VM's CodeCache is keyed by source text, so every reload of an edited
// file inserts a new UnlinkedModuleProgramCodeBlock entry that holds a
// Strong<> to the old SourceProvider (and its source string). Its prune
// policy only kicks in after ~10s elapsed or ~16 MB of accumulated source,
// which a tight edit-save loop never hits, so stale entries pile up
// unbounded. We already re-transpile every module after clearAll() above,
// so re-parsing cost here is marginal.
Comment thread
robobun marked this conversation as resolved.
Outdated
vm.codeCache()->clear();

// If we run the GC every time, we will never get the SourceProvider cache hit.
// So we run the GC every other time.
if ((this->reloadCount++ + 1) % 2 == 0) {
Expand Down
89 changes: 89 additions & 0 deletions test/regression/issue/11083.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
// https://github.com/oven-sh/bun/issues/11083
//
// Each --hot reload of an edited file inserted a new UnlinkedModuleProgramCodeBlock
// into JSC's CodeCache (keyed by source text) that holds a Strong<> to the
// SourceProvider and its source string. The cache's prune only fires after ~10s
// elapsed or ~16 MB accumulated, so a tight edit loop piled up stale entries
// unbounded. On top of that, the ref_strings source cache leaked its initial +1
// ref so the duped source bytes survived even after the provider was collected.
// reload() now clears the CodeCache and the ref is balanced.

import { spawn } from "bun";
import { expect, test } from "bun:test";
import { bunEnv, bunExe, isDebug, tempDir } from "harness";
import { writeFileSync } from "node:fs";
import { join } from "node:path";

test(
"bun --hot should not accumulate stale code blocks when file content changes on every reload",
async () => {
using dir = tempDir("hot-11083", {});
const root = join(String(dir), "leak-runner.mjs");

// Keep the file small so even a debug build cycles fast enough to outrun
// JSC's ~10s CodeCache prune timer. Content is unique every iteration so
// the cache key (source hash) never repeats.
const writeSource = (iter: number) => {
writeFileSync(
root,
`var unused_${iter} = ${iter};
Bun.gc(true);
globalThis.__i = (globalThis.__i ?? 0) + 1;
const s = require("bun:jsc").heapStats();
console.error(JSON.stringify({
i: globalThis.__i,
umpcb: s.objectTypeCounts.UnlinkedModuleProgramCodeBlock || 0,
}));
`,
);
};

let iter = 0;
writeSource(++iter);

await using runner = spawn({
cmd: [bunExe(), "--hot", "run", root],
env: bunEnv,
cwd: String(dir),
stdout: "ignore",
stderr: "pipe",
stdin: "ignore",
});

const target = 50;
let maxCodeBlocks = 0;
let reached = 0;
let buf = "";
outer: for await (const chunk of runner.stderr!) {
buf += new TextDecoder().decode(chunk);
let nl: number;
let progressed = false;
while ((nl = buf.indexOf("\n")) >= 0) {
const line = buf.slice(0, nl);
buf = buf.slice(nl + 1);
if (!line.startsWith("{")) continue;
const { i, umpcb } = JSON.parse(line);
reached = i;
maxCodeBlocks = Math.max(maxCodeBlocks, umpcb);
progressed = true;
if (i >= target) {
runner.kill();
break outer;
}
}
if (progressed) writeSource(++iter);
}

expect(reached).toBe(target);

// With the CodeCache cleared on every reload only the current module
// graph's blocks survive a sync GC (bun:main + this file = 2). Without it,
// one entry is added per reload and none are evicted inside the first
// ~10s, so this climbs to ~target. The leaked code block pins its
// SourceProvider and source string, so this single count covers the whole
// chain; a heap-wide JSString delta would be noisier under conservative
// GC / JIT tier-up without adding coverage.
expect(maxCodeBlocks).toBeLessThan(10);
},
isDebug ? 60_000 : 20_000,
);
Loading