From 2cf9f0aaeddbe3f0ae7bae6f20982a5989696972 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 12 Aug 2026 03:05:29 +0000 Subject: [PATCH 1/4] bundler: hand a finished Bun.build back through its pointer, not a &mut receiver complete_on_bundle_thread posts the completion task's only ref to the JS thread, which frees the task as soon as the post lands. It took &mut self, and BundleThread held the task as &mut C across the post as well, so the allocation could be freed while reference arguments to it were still live. The trait method now takes *mut Self and BundleThread keeps the dequeued pointer raw, reborrowing it per call; nothing forms a reference to the task across either hand-back. Adds a source lint banning posting a pointer spelled from self as a task, with the remaining same-shaped sites ratcheted. --- src/bundler/BundleThread.rs | 91 ++++-- src/runtime/api/js_bundle_completion_task.rs | 22 +- .../self-receiver-publish.test.ts | 260 ++++++++++++++++++ 3 files changed, 338 insertions(+), 35 deletions(-) create mode 100644 test/internal/source-lints/self-receiver-publish.test.ts diff --git a/src/bundler/BundleThread.rs b/src/bundler/BundleThread.rs index cef2a0487354..3563ea273dc9 100644 --- a/src/bundler/BundleThread.rs +++ b/src/bundler/BundleThread.rs @@ -69,7 +69,18 @@ pub trait CompletionStruct: Node + Send + 'static { /// while it was still queued ([`free_released_unstarted`] then frees it). fn try_start(&mut self) -> bool; fn free_released_unstarted(this: *mut Self); - fn complete_on_bundle_thread(&mut self); + /// Hands the finished build (result and log already stored) back to its + /// owner, which may free `*this` the moment it is posted: this is the + /// bundle thread's last touch of it. It takes the pointer rather than + /// `&mut self` because a reference argument stays protected until the + /// call returns, and freeing protected memory is UB under both aliasing + /// models; for the same reason the caller must not hold a reference to + /// `*this` across the call either. + /// + /// # Safety + /// `this` is a build that `try_start` accepted and the bundle thread still + /// owns; nothing on this thread touches `*this` afterwards. + unsafe fn complete_on_bundle_thread(this: *mut Self); fn set_result(&mut self, result: BundleV2Result); fn set_log(&mut self, log: bun_ast::Log); fn set_transpiler(&mut self, this: *mut BundleV2<'_>); @@ -225,24 +236,28 @@ impl BundleThread { break; } // SAFETY: queue stores non-null *mut C pushed via enqueue(); owner keeps it alive - // until complete_on_bundle_thread() signals completion — unless it + // until complete_on_bundle_thread() hands it back — unless it // released the build while it sat here (its VM went away). if !unsafe { (*completion).try_start() } { C::free_released_unstarted(completion); continue; } - // SAFETY: as above; started ⇒ the owner waits for us. - let completion = unsafe { &mut *completion }; // SAFETY: `generation` is only read/written on this (bundle) thread. let generation = unsafe { (*instance).generation }; // `panic = "abort"` → a Rust panic on this thread enters the // crash-handler hook and aborts the whole process. // No `catch_unwind` — there is nothing to catch. - match Self::generate_in_new_thread(completion, generation) { - Ok(()) => {} - Err(err) => { - completion.set_result(BundleV2Result::Err(err)); - completion.complete_on_bundle_thread(); + // + // SAFETY: started ⇒ `*completion` is ours until it is handed + // back, and handing it back is the last thing either path does + // with it. The owner may free it as soon as it is posted, so it + // stays a raw pointer: `set_result`'s reborrow ends at the `;` + // and no reference to it is live across either hand-back (see + // `complete_on_bundle_thread`). + unsafe { + if let Err(err) = Self::generate_in_new_thread(completion, generation) { + (*completion).set_result(BundleV2Result::Err(err)); + C::complete_on_bundle_thread(completion); } } has_bundled = true; @@ -264,8 +279,16 @@ impl BundleThread { } /// This is called from `Bun.build` in JavaScript. - fn generate_in_new_thread( - completion: &mut C, + /// + /// On `Ok` the build has been handed back (`complete_on_bundle_thread`) + /// and `*completion` may already be gone; on `Err` it is still ours and + /// the caller stores the error and hands it back. + /// + /// # Safety + /// `completion` is a build that `try_start` accepted and the bundle thread + /// still owns. + unsafe fn generate_in_new_thread( + completion: *mut C, generation: bun_core::Generation, ) -> Result<(), crate::Error> { let heap = Arena::new(); @@ -277,22 +300,28 @@ impl BundleThread { ast_memory_store.push(); // Allocate + configure folded — see `create_and_configure_transpiler` doc. - let transpiler = completion.create_and_configure_transpiler(bump)?; + // + // SAFETY: fn contract. `*completion` is reborrowed for this one call; + // the returned `&'a mut Transpiler` borrows `bump`, not the task. + let transpiler = unsafe { (*completion).create_and_configure_transpiler(bump) }?; transpiler.resolver.generation = generation; - // Construction + run delegated — see - // `init_and_run` doc. Reborrow `transpiler` through a raw ptr so - // `completion` can be borrowed again below. + // Construction + run delegated — see `init_and_run` doc. Keep a raw + // ptr to `transpiler`: `init_and_run` consumes the `&'a mut`, and the + // log is read (and the struct dropped) through it afterwards. let transpiler_ptr: *mut Transpiler<'_> = transpiler; - let run = completion.init_and_run( - // SAFETY: `transpiler` lives in `bump` for the duration of `heap`. - unsafe { &mut *transpiler_ptr }, - bump, - // `WorkPool::get()` returns `&'static ThreadPool`; pass as raw so - // the impl can hand it to `BundleV2::init` (which stores `*mut`). - std::ptr::from_ref(bun_threading::work_pool::WorkPool::get()).cast_mut(), - ); + // SAFETY: fn contract (reborrowed for this one call, as above); + // `transpiler` lives in `bump` for the duration of `heap`. + let run = unsafe { + (*completion).init_and_run( + &mut *transpiler_ptr, + bump, + // `WorkPool::get()` returns `&'static ThreadPool`; pass as raw so + // the impl can hand it to `BundleV2::init` (which stores `*mut`). + std::ptr::from_ref(bun_threading::work_pool::WorkPool::get()).cast_mut(), + ) + }; // Straight-line teardown: log copy // runs on both paths; `completeOnBundleThread` only on success (the error @@ -300,14 +329,20 @@ impl BundleThread { // `deinitWithoutFreeingArena` + wait-group drain live inside `init_and_run` // (it owns `this`). let mut out_log = bun_ast::Log::init(); - // SAFETY: `transpiler.log` is the arena-allocated `*mut Log` set up by - // `configure_bundler`; valid for the lifetime of `heap`. Raw deref so the - // `&'a mut Transpiler` consumed by `init_and_run` above is not reborrowed. + // SAFETY: `transpiler.log` is the `*mut Log` that + // `create_and_configure_transpiler` installed (the one impl points it + // into the task, which is still ours here). Raw deref so the `&'a mut + // Transpiler` consumed by `init_and_run` above is not reborrowed. let _ = unsafe { (*(*transpiler_ptr).log).append_to_with_recycled(&mut out_log, true) }; // logger OOM-only - completion.set_log(out_log); + // SAFETY: fn contract; reborrowed for this one call. + unsafe { (*completion).set_log(out_log) }; if run.is_ok() { - completion.complete_on_bundle_thread(); + // SAFETY: fn contract; the result and log are stored, the + // reborrows above have ended, and nothing below touches + // `*completion` (or, through `transpiler.log`, the task's log): the + // teardown drops only memory this thread owns. + unsafe { C::complete_on_bundle_thread(completion) }; } ast_memory_store.pop(); diff --git a/src/runtime/api/js_bundle_completion_task.rs b/src/runtime/api/js_bundle_completion_task.rs index 27657f0da198..f5d61f76e8a0 100644 --- a/src/runtime/api/js_bundle_completion_task.rs +++ b/src/runtime/api/js_bundle_completion_task.rs @@ -561,8 +561,8 @@ impl JSBundleCompletionTask { pub(crate) fn on_complete_anytask(ctx: *mut Self) -> bun_event_loop::JsResult<()> { crate::jsc_hooks::ActiveHandle::Bundle(NonNull::new(ctx).expect("completion")).unregister(); - // For the +1 taken by `complete_on_bundle_thread` enqueue. - // SAFETY: `ctx` is the live heap allocation; `adopt` consumes the prior +1 on Drop. + // The creation ref, which `complete_on_bundle_thread` posted back to us. + // SAFETY: `ctx` is the live heap allocation; `adopt` consumes that ref on Drop. let _drop_ref = unsafe { bun_ptr::ScopedRef::::adopt(ctx) }; // SAFETY: `ctx` is the heap::alloc allocation registered in `task`, // dispatched exactly once per task on the JS thread. Exclusive: the @@ -1114,13 +1114,21 @@ impl CompletionStruct for JSBundleCompletionTask { Ok(()) } - fn complete_on_bundle_thread(&mut self) { + unsafe fn complete_on_bundle_thread(this: *mut Self) { // The bundle thread's last touch of this task and of the VM's memory: // hand it back (always queued — the VM waits for it) and stop counting. - self.bundle_loop - .store(ptr::null_mut(), core::sync::atomic::Ordering::Release); - let handle = self.loop_handle.clone(); - let this = std::ptr::from_mut::(self); + // The post carries the creation ref, which `on_complete_anytask` + // releases, so the JS thread may free `*this` as soon as the post + // lands: everything needed afterwards is taken out first, through + // accesses that end at their `;`. + // + // SAFETY: trait contract — `this` is the started build, still ours. + let handle = unsafe { + (*this) + .bundle_loop + .store(ptr::null_mut(), core::sync::atomic::Ordering::Release); + (*this).loop_handle.clone() + }; let ct = jsc::ConcurrentTask::create(jsc::Task::init(this)); let jsc::vm_handle::Posted::Queued = handle.post_task(ct) else { unreachable!("VM handle closed with a Bun.build outstanding"); diff --git a/test/internal/source-lints/self-receiver-publish.test.ts b/test/internal/source-lints/self-receiver-publish.test.ts new file mode 100644 index 000000000000..577d8baed16a --- /dev/null +++ b/test/internal/source-lints/self-receiver-publish.test.ts @@ -0,0 +1,260 @@ +import { file } from "bun"; +import { expect, test } from "bun:test"; +import { realpathSync } from "fs"; +import path from "path"; +import { globAllSources } from "../../../scripts/glob-sources.ts"; + +// A method must not post its own receiver as a task. Inside a `&self` / +// `&mut self` method, a pointer spelled from `self` +// +// ConcurrentTask::create(Task::init(std::ptr::from_mut(self))) +// ConcurrentTask::create_from(self) // `&mut Self` coerces to `*mut Self` +// let this = std::ptr::from_mut(self); ... Task::init(this) +// let p: *mut Self = self; ... ConcurrentTask::create_from(p) +// +// as the argument of `Task::init(..)` / `..::create_from(..)` / +// `..::from_callback(..)` is banned. +// +// The post is the hand-over. From the moment it lands, the consumer (another +// thread, for a `ConcurrentTask`) may write to the object or, when the post +// carries its last ref, free it, and it does so while `self` is still a live +// argument of the method that posted it. A reference argument is protected +// for the duration of its call, and writing to or deallocating protected +// memory is UB under both aliasing models whether or not the method touches +// `self` again: Tree Borrows (what `bun run rust:miri` uses) reports +// "deallocation through is forbidden ... the strongly protected tag +// disallows deallocations", pointing at the receiver, and Stacked Borrows +// reports "deallocating while item [Unique] is strongly protected". Codegen +// relies on the same guarantee: a reference argument is annotated as +// dereferenceable for the whole call. `JSBundleCompletionTask:: +// complete_on_bundle_thread(&mut self)` was the instance this was written +// for: every `Bun.build()` ended with the bundle thread posting the task's +// only ref to the JS thread, which frees it, from inside its own `&mut self`. +// +// The object was a raw pointer in the caller's hands before it became `self` +// (the queue entry, the callback ctx, the backref), so the fix is to keep it +// one: the function that posts takes `this: *mut Self`, does its own work +// through accesses that end before the post (`(*this).field` place +// expressions or a call-scoped reborrow), and posts `this`. Templates: +// `complete_on_bundle_thread` in src/runtime/api/js_bundle_completion_task.rs +// (and its caller in src/bundler/BundleThread.rs, which keeps the pointer +// raw), `async_job_run` in src/runtime/node/node_zlib_binding.rs, `post_job` +// in src/jsc/VmHandle.rs. +// +// Scope: the spellings above, with `self` as the receiver and the three task +// constructors as the callee. A pointer produced by a helper (`self.as_ptr()`), +// a `NonNull::from(self)` binding posted through `.as_ptr()`, the intrusive +// `ConcurrentTask::from(..)` form, and reference *parameters* other than +// `self` (`fn f(load: &mut Load)` posting `from_mut(load)`) are the same +// hazard but outside this lint. Siblings: self-receiver-reclaim.test.ts, +// fn-long-mut-reborrow.test.ts, frozen-nonnull-reborrow.test.ts. + +const root = path.resolve(import.meta.dir, "..", "..", ".."); +const rustSources = globAllSources().rust.filter(p => p.endsWith(".rs")); + +// Only scan files tracked in HEAD (a `git stash` round-trip can leave stray +// `.rs` files in the working tree; CI runs on a clean checkout). Same guard as +// dead-code-escapes.test.ts. +const tracked: Set | null = (() => { + const r = Bun.spawnSync({ + cmd: ["git", "-C", root, "ls-tree", "-r", "--name-only", "-z", "HEAD"], + stdout: "pipe", + stderr: "ignore", + }); + if (!r.success) return null; + return new Set(r.stdout.toString().split("\0").filter(Boolean)); +})(); + +// `Task::init(`, `ConcurrentTask::create_from(`, `ConcurrentTask::from_callback(`, +// however the path in front is spelled, optionally turbofished. The `\b` +// before `Task` keeps `NapiFinalizerTask::init(self)` (a constructor that +// copies out of `self`) out; `create_from` / `from_callback` require a path +// in front so a method call of the same name does not count. `\s*` after the +// paren so a rustfmt-wrapped argument still matches. +const POST = String.raw`\b(?:Task::init|[\w:]+::create_from|[\w:]+::from_callback)(?:::<[^>]*>)?\(\s*`; + +// The ways of spelling "`self`, as a raw pointer" as the first argument. Each +// is anchored at its front only, so a trailing `.cast_mut()` / `.as_ptr()` +// still matches; the bare form needs the `,` / `)` so `self.as_ptr()` (a +// helper, out of scope) does not. `(?!\s*\.)` after the `&raw` form keeps +// `&raw mut *self.field` (a field the receiver owns) out. +const SELF_AS_POINTER = [ + String.raw`self\s*[,)]`, + String.raw`(?:[\w:]+::)?from_(?:mut|ref)(?:::<[^>]*>)?\(\s*self\s*\)`, + String.raw`(?:[\w:]+::)?NonNull::from\(\s*self\s*\)`, + String.raw`self\s+as\s+\*(?:mut|const)\b`, + String.raw`&raw\s+(?:mut|const)\s+\*\s*self\b(?!\s*\.)`, + String.raw`(?:[\w:]+::)?addr_of(?:_mut)?!\s*\(\s*\*\s*self\s*\)`, +].join("|"); + +const DIRECT = new RegExp(`${POST}(?:${SELF_AS_POINTER})`, "g"); + +// A local bound to such a pointer: `let this = ptr::from_mut(self);`, +// `let p = self as *mut Self;`, `let p: *mut Self = self;` (the coercion +// spelling needs the annotation; without it `let p = self;` is just another +// reference). The binding is then looked for as a post argument further down +// the same function, which ends at the next `fn` item (a closure inside it is +// the same function for this purpose). +const BINDING_HEAD = String.raw`let\s+(?:mut\s+)?(\w+)\s*`; +const SELF_POINTER_BINDINGS = [ + new RegExp( + BINDING_HEAD + + String.raw`(?::[^=;]*)?=\s*(?:` + + [ + String.raw`(?:[\w:]+::)?from_(?:mut|ref)(?:::<[^>]*>)?\(\s*self\s*\)(?:\s*\.cast_mut\(\))?`, + String.raw`self\s+as\s+\*(?:mut|const)\b[^;]*`, + String.raw`&raw\s+(?:mut|const)\s+\*\s*self\b`, + String.raw`(?:[\w:]+::)?addr_of(?:_mut)?!\s*\(\s*\*\s*self\s*\)`, + ].join("|") + + String.raw`)\s*;`, + "g", + ), + new RegExp(BINDING_HEAD + String.raw`:\s*\*(?:mut|const)\b[^=;]*=\s*self\s*;`, "g"), +]; +const FN_ITEM = /^[ \t]*(?:pub(?:\([^)]*\))?\s+)?(?:(?:const|async|unsafe|extern\s+"[^"]*")\s+)*fn\s/m; + +function postOfBinding(name: string): RegExp { + return new RegExp(POST + name + String.raw`\s*[,)]`); +} + +/** Byte offsets (into `stripped`) of every banned post in one file. */ +function findPosts(stripped: string): number[] { + const hits: number[] = []; + for (const m of stripped.matchAll(DIRECT)) hits.push(m.index); + for (const pattern of SELF_POINTER_BINDINGS) { + for (const binding of stripped.matchAll(pattern)) { + const start = binding.index + binding[0].length; + const rest = stripped.slice(start); + const fnEnd = rest.search(FN_ITEM); + const body = fnEnd === -1 ? rest : rest.slice(0, fnEnd); + const post = body.search(postOfBinding(binding[1])); + if (post !== -1) hits.push(start + post); + } + } + return hits.sort((a, b) => a - b); +} + +function lineOf(text: string, offset: number): number { + return text.slice(0, offset).split("\n").length; +} + +// Documented, ratcheted exceptions: files allowed to keep exactly N of the +// shape. Each has been read; none frees the receiver from the consumer's side, +// and each needs a change elsewhere before its own receiver can be converted. +// Lower an entry when you convert one; do not add entries. +const ALLOW: Record = { + // `Resolve::dispatch` / `Load::dispatch` (`&mut self`) post the arena-owned + // plugin request to the JS thread, which writes its `value` while + // `dispatch` is still returning; the arena, not the consumer, frees it. + // Converting them means `dispatch(this: *mut Self)` with the arena pointer + // the callers already hold; tracked separately. + "src/bundler/bundle_v2.rs": 2, + // `DeferredBatchTask::schedule` (`&mut self`) posts a task embedded in its + // `BundleV2`, whose consumer reaches the surrounding `BundleV2` through it; + // the receiver that matters there is the `&mut BundleV2` the bundle thread + // holds for the whole pass, not this field's. #37709 reshapes `schedule` to + // take the `BundleV2`; delete this entry when that lands. + "src/bundler/DeferredBatchTask.rs": 1, + // `ThreadSafeFunction::maybe_queue_finalizer` posts to the loop it is + // running on, so the task runs after the method has returned. + // `schedule_dispatch` posts from addon threads and the JS thread starts + // writing the TSFN's atomics at once, but its callers (`call`, + // `release_locked`) hold `&mut self` across it too, so it cannot be + // converted on its own; tracked separately. + "src/runtime/napi/napi_body.rs": 2, +}; + +const counts: Record = {}; +const offenders: string[] = []; +let scanned = 0; +for (const abs of rustSources) { + const source = path.relative(root, abs).replaceAll(path.sep, "/"); + // `src/cli` is a symlink into `src/runtime/cli`; count each file once under + // its canonical path. + if (path.relative(root, realpathSync(abs)).replaceAll(path.sep, "/") !== source) continue; + if (tracked !== null && !tracked.has(source)) continue; + scanned++; + const content = await file(abs).text(); + // Strip full-line comments so prose mentions (including the in-tree comments + // describing this hazard) don't count. `[ \t]*`, not `\s*`: `\s` crosses + // newlines and would swallow blank lines, shifting the reported line numbers. + const stripped = content.replace(/^[ \t]*\/\/.*$/gm, ""); + for (const offset of findPosts(stripped)) { + counts[source] = (counts[source] ?? 0) + 1; + if (counts[source] > (ALLOW[source] ?? 0)) { + offenders.push(`${source}:${lineOf(stripped, offset)}`); + } + } +} + +test("scans a non-empty set of tracked Rust sources", () => { + // Guards against the tracked/realpath filters above over-firing and leaving + // nothing to scan, which would make the ban below pass vacuously. + expect(scanned).toBeGreaterThan(0); +}); + +test("the patterns match the banned spellings and nothing else", () => { + const banned = [ + // `complete_on_bundle_thread(&mut self)` as it was. + "let this = std::ptr::from_mut::(self);\nlet ct = jsc::ConcurrentTask::create(jsc::Task::init(this));", + // The allowlisted shapes. + "ConcurrentTask::create(Task::init(std::ptr::from_mut::(self)));", + "bun_event_loop::ConcurrentTask::ConcurrentTask::create(\n bun_event_loop::Task::init(std::ptr::from_mut::(self)),\n);", + "let self_ptr: *mut Self = self;\nif done {\n return;\n}\nlet ct = ConcurrentTask::create_from(self_ptr);", + "let self_ptr: *mut Self = self;\nloop_.enqueue_task(Task::init(self_ptr));", + // Other spellings of the pointer. + "loop_.enqueue_task(Task::init(self));", + "let ct = ConcurrentTask::create_from(self);", + "ConcurrentTask::from_callback(self, Self::resume);", + "ConcurrentTask::from_callback(\n std::ptr::from_mut(self),\n Self::resume,\n)", + "Task::init(core::ptr::from_ref(self).cast_mut())", + "Task::init(NonNull::from(self).as_ptr())", + "Task::init(self as *mut Self)", + "Task::init(&raw mut *self)", + "Task::init(core::ptr::addr_of_mut!(*self))", + "jsc::ConcurrentTask::create_from::(std::ptr::from_mut(self))", + "let p = self as *mut Self;\nlet ct = ConcurrentTask::create_from(p);", + "let p: *mut Self = std::ptr::from_mut(self);\nlet ct = bun_jsc::ConcurrentTask::create_from(p);", + "let p = core::ptr::from_ref(self).cast_mut();\nlet ct = ConcurrentTask::create_from(p);", + "let p = &raw mut *self;\nlet task = Task::init(p);", + ]; + const allowed = [ + // Posting the pointer the caller handed us is the intended shape. + "let ct = jsc::ConcurrentTask::create(jsc::Task::init(this));", + "let ct = ConcurrentTask::create(Task::init(this));", + "ConcurrentTask::create_from(task.as_ptr())", + "ConcurrentTask::from_callback(this, FetchTasklet::resume_request_data_stream)", + // Something the receiver owns or points at, or a helper's pointer, is + // out of scope (see the header). + "ConcurrentTask::create(Task::init(self.as_ctx_ptr()))", + "Task::init(core::ptr::from_ref(&self.run_pending_later).cast_mut())", + "Task::init(&raw mut *self.inner)", + "Task::init(self.task)", + "ConcurrentTask::from_callback(std::ptr::from_mut(load), on_load_from_js_loop_raw)", + // A constructor whose name merely ends in `Task`, and a method call. + "NapiFinalizerTask::init(self).schedule();", + "state.create_from(index, from, into);", + // Producing the pointer is fine when it is not what gets posted. + "let this = std::ptr::from_mut::(self);\nregister(this);", + "let this = std::ptr::from_mut(self);\nlet ct = ConcurrentTask::create_from(other);", + // Rebinding the reference is not a pointer binding. + "let this = self;\nlet ct = ConcurrentTask::create_from(this);", + // The binding is posted, but in the next function, where it is a + // raw-pointer parameter of the same name. + "let this = std::ptr::from_mut(self);\nregister(this);\n}\n\nfn resume(this: *mut Self) {\n let ct = ConcurrentTask::create_from(this);\n}", + ]; + expect(banned.map(s => findPosts(s).length)).toEqual(banned.map(() => 1)); + expect(allowed.map(s => findPosts(s).length)).toEqual(allowed.map(() => 0)); +}); + +test("no method posts its own receiver as a task", () => { + expect(offenders).toEqual([]); +}); + +test("allowlisted files still carry exactly their documented count", () => { + // Ratchet: when an allowlisted site is converted, lower its entry so the + // shape cannot come back into that file. + for (const [source, n] of Object.entries(ALLOW)) { + expect({ source, count: counts[source] ?? 0 }).toEqual({ source, count: n }); + } +}); From 3644c91612d9a536a003929a4a640525f0c8a131 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 12 Aug 2026 07:37:07 +0000 Subject: [PATCH 2/4] source-lints: cover the intrusive .from() post and document what self-receiver-publish cannot see Pin create_from/from_callback to ConcurrentTask paths, add the intrusive .from(..) spelling, state the same-thread (provenance) hazard that makes same-loop posts part of the shape, list the same hand-over spelled outside the regex, and correct the napi allowlist note: the finalizer post's consumer does free the receiver. --- .../self-receiver-publish.test.ts | 132 ++++++++++++------ 1 file changed, 88 insertions(+), 44 deletions(-) diff --git a/test/internal/source-lints/self-receiver-publish.test.ts b/test/internal/source-lints/self-receiver-publish.test.ts index 577d8baed16a..8cdcb274c31a 100644 --- a/test/internal/source-lints/self-receiver-publish.test.ts +++ b/test/internal/source-lints/self-receiver-publish.test.ts @@ -9,27 +9,42 @@ import { globAllSources } from "../../../scripts/glob-sources.ts"; // // ConcurrentTask::create(Task::init(std::ptr::from_mut(self))) // ConcurrentTask::create_from(self) // `&mut Self` coerces to `*mut Self` +// loop_.enqueue_task(Task::init(self)) // same loop, drained later // let this = std::ptr::from_mut(self); ... Task::init(this) // let p: *mut Self = self; ... ConcurrentTask::create_from(p) // -// as the argument of `Task::init(..)` / `..::create_from(..)` / -// `..::from_callback(..)` is banned. +// as the argument of `Task::init(..)`, `ConcurrentTask::create_from(..)`, +// `ConcurrentTask::from_callback(..)` or the intrusive `.from(..)` is banned. // -// The post is the hand-over. From the moment it lands, the consumer (another -// thread, for a `ConcurrentTask`) may write to the object or, when the post -// carries its last ref, free it, and it does so while `self` is still a live -// argument of the method that posted it. A reference argument is protected -// for the duration of its call, and writing to or deallocating protected -// memory is UB under both aliasing models whether or not the method touches -// `self` again: Tree Borrows (what `bun run rust:miri` uses) reports -// "deallocation through is forbidden ... the strongly protected tag -// disallows deallocations", pointing at the receiver, and Stacked Borrows -// reports "deallocating while item [Unique] is strongly protected". Codegen -// relies on the same guarantee: a reference argument is annotated as -// dereferenceable for the whole call. `JSBundleCompletionTask:: -// complete_on_bundle_thread(&mut self)` was the instance this was written -// for: every `Bun.build()` ended with the bundle thread posting the task's -// only ref to the JS thread, which frees it, from inside its own `&mut self`. +// Two things are wrong with it, and which one bites depends on who drains the +// queue: +// +// - Another thread (a `ConcurrentTask`): the post is the hand-over. From the +// moment it lands the consumer may write to the object or, when the post +// carries its last ref, free it, while `self` is still a live argument of +// the posting method. A reference argument is protected for the duration +// of its call, and writing to or deallocating protected memory is UB under +// both aliasing models whether or not the method touches `self` again: +// Tree Borrows (what `bun run rust:miri` uses) reports "deallocation +// through is forbidden ... the strongly protected tag disallows +// deallocations", pointing at the receiver, and Stacked Borrows reports +// "deallocating while item [Unique] is strongly protected". Codegen relies +// on the same guarantee: a reference argument is annotated as +// dereferenceable for the whole call. +// - The same thread, later: the queued pointer carries the provenance of the +// receiver reborrow it was spelled from, and that reborrow is dead as soon +// as anything reaches the object through the owner's own pointer again (the +// caller that holds it, the next event, a state CAS on the way out of the +// dispatch). When the queue is drained the task is read, or freed, through +// a dead pointer: "reborrow through is forbidden" (Tree Borrows) / +// "that tag does not exist in the borrow stack" (Stacked Borrows). This is +// why `SendQueue` in src/runtime/ipc.rs keeps its allocation pointer in a +// field and posts `root_ptr()`. +// +// `JSBundleCompletionTask::complete_on_bundle_thread(&mut self)` was the +// instance this was written for: every `Bun.build()` ended with the bundle +// thread posting the task's only ref to the JS thread, which frees it, from +// inside its own `&mut self`. // // The object was a raw pointer in the caller's hands before it became `self` // (the queue entry, the callback ctx, the backref), so the fix is to keep it @@ -39,14 +54,23 @@ import { globAllSources } from "../../../scripts/glob-sources.ts"; // `complete_on_bundle_thread` in src/runtime/api/js_bundle_completion_task.rs // (and its caller in src/bundler/BundleThread.rs, which keeps the pointer // raw), `async_job_run` in src/runtime/node/node_zlib_binding.rs, `post_job` -// in src/jsc/VmHandle.rs. +// in src/jsc/VmHandle.rs, the `(*this).concurrent_task.from(this, ..)` posts +// in src/runtime/webcore/s3/. // -// Scope: the spellings above, with `self` as the receiver and the three task -// constructors as the callee. A pointer produced by a helper (`self.as_ptr()`), -// a `NonNull::from(self)` binding posted through `.as_ptr()`, the intrusive -// `ConcurrentTask::from(..)` form, and reference *parameters* other than -// `self` (`fn f(load: &mut Load)` posting `from_mut(load)`) are the same -// hazard but outside this lint. Siblings: self-receiver-reclaim.test.ts, +// Scope: the spellings above, with `self` as the receiver and those four +// callees. The same hand-over spelled some other way is outside the regex and +// is the same bug; the instances known at the time of writing, each tracked +// for its own conversion, are `napi_async_work::run` / `post_to_js_thread` in +// src/runtime/napi/napi_body.rs (posts a `*mut Self` parameter the caller made +// from its `&mut self`), `FetchTasklet::deref_from_thread` in +// src/runtime/webcore/fetch/FetchTasklet.rs (posts `this`, but through +// `post(&self)` and the handle field inside the object it is freeing), +// `StatWatcher::post_to_js_thread(&self)` in src/runtime/node/ +// node_fs_stat_watcher.rs (a helper's pointer, `self.as_ctx_ptr()`), and +// `TranspilerJob::dispatch_to_main_thread(&mut self)` in +// src/jsc/RuntimeTranspilerStore.rs (pushes `NonNull::from(&mut *self)` onto +// its own queue). Convert anything of that shape on sight; the ratchet below +// only tracks what the regex can see. Siblings: self-receiver-reclaim.test.ts, // fn-long-mut-reborrow.test.ts, frozen-nonnull-reborrow.test.ts. const root = path.resolve(import.meta.dir, "..", "..", ".."); @@ -66,12 +90,15 @@ const tracked: Set | null = (() => { })(); // `Task::init(`, `ConcurrentTask::create_from(`, `ConcurrentTask::from_callback(`, -// however the path in front is spelled, optionally turbofished. The `\b` -// before `Task` keeps `NapiFinalizerTask::init(self)` (a constructor that -// copies out of `self`) out; `create_from` / `from_callback` require a path -// in front so a method call of the same name does not count. `\s*` after the -// paren so a rustfmt-wrapped argument still matches. -const POST = String.raw`\b(?:Task::init|[\w:]+::create_from|[\w:]+::from_callback)(?:::<[^>]*>)?\(\s*`; +// however the path in front is spelled (`jsc::`, `bun_event_loop::ConcurrentTask::`, +// the `ConcurrentTaskItem` alias), optionally turbofished, plus the intrusive +// `.from(` of an embedded `ConcurrentTask` / `AnyTask` (the tree's only +// method-call `.from(`). The `\b` before `Task` keeps `NapiFinalizerTask:: +// init(self)` (a constructor that copies out of `self`) out, and pinning the +// other two to a `ConcurrentTask*` path keeps unrelated `create_from` +// constructors out. `\s*` after the paren so a rustfmt-wrapped argument still +// matches. +const POST = String.raw`(?:\bTask::init|\bConcurrentTask\w*::(?:create_from|from_callback)|\.from)(?:::<[^>]*>)?\(\s*`; // The ways of spelling "`self`, as a raw pointer" as the first argument. Each // is anchored at its front only, so a trailing `.cast_mut()` / `.as_ptr()` @@ -139,15 +166,16 @@ function lineOf(text: string, offset: number): number { } // Documented, ratcheted exceptions: files allowed to keep exactly N of the -// shape. Each has been read; none frees the receiver from the consumer's side, -// and each needs a change elsewhere before its own receiver can be converted. -// Lower an entry when you convert one; do not add entries. +// shape. Each has been read and is the bug described above, not a false +// positive; each is listed because its conversion is a change of its own, +// tracked separately, not because it is safe. Lower an entry when you convert +// one; do not add entries. const ALLOW: Record = { // `Resolve::dispatch` / `Load::dispatch` (`&mut self`) post the arena-owned // plugin request to the JS thread, which writes its `value` while - // `dispatch` is still returning; the arena, not the consumer, frees it. - // Converting them means `dispatch(this: *mut Self)` with the arena pointer - // the callers already hold; tracked separately. + // `dispatch` is still returning (the arena, not the consumer, frees it). + // Conversion: `dispatch(this: *mut Self)` with the pointer the callers + // already hold. "src/bundler/bundle_v2.rs": 2, // `DeferredBatchTask::schedule` (`&mut self`) posts a task embedded in its // `BundleV2`, whose consumer reaches the surrounding `BundleV2` through it; @@ -155,12 +183,14 @@ const ALLOW: Record = { // holds for the whole pass, not this field's. #37709 reshapes `schedule` to // take the `BundleV2`; delete this entry when that lands. "src/bundler/DeferredBatchTask.rs": 1, - // `ThreadSafeFunction::maybe_queue_finalizer` posts to the loop it is - // running on, so the task runs after the method has returned. - // `schedule_dispatch` posts from addon threads and the JS thread starts - // writing the TSFN's atomics at once, but its callers (`call`, - // `release_locked`) hold `&mut self` across it too, so it cannot be - // converted on its own; tracked separately. + // `ThreadSafeFunction::maybe_queue_finalizer` is the same-thread case: it + // sets `closing` and posts `self_ptr` to its own loop, `on_dispatch` then + // CASes `dispatch_state` through the real pointer on its way out, and the + // drained task is what `destroy`s the TSFN, through the dead one. + // `schedule_dispatch` is the cross-thread case (addon threads post, the JS + // thread writes the atomics at once). Both are reached through `&mut self` + // callers (`dispatch_one`; `call` / `release_locked`) that have to be + // converted with them. "src/runtime/napi/napi_body.rs": 2, }; @@ -202,7 +232,12 @@ test("the patterns match the banned spellings and nothing else", () => { "bun_event_loop::ConcurrentTask::ConcurrentTask::create(\n bun_event_loop::Task::init(std::ptr::from_mut::(self)),\n);", "let self_ptr: *mut Self = self;\nif done {\n return;\n}\nlet ct = ConcurrentTask::create_from(self_ptr);", "let self_ptr: *mut Self = self;\nloop_.enqueue_task(Task::init(self_ptr));", - // Other spellings of the pointer. + // The intrusive form. + "let ct = self.concurrent_task.from(std::ptr::from_mut(self), AutoDeinit::ManualDeinit);", + "let self_ptr: *mut Self = self;\nlet ct = self.concurrent_task.from(self_ptr, AutoDeinit::ManualDeinit);", + "at.from(self, Self::run_from_main_thread_mini)", + // Other spellings of the pointer; a same-loop post counts too (see the + // header). "loop_.enqueue_task(Task::init(self));", "let ct = ConcurrentTask::create_from(self);", "ConcurrentTask::from_callback(self, Self::resume);", @@ -213,6 +248,8 @@ test("the patterns match the banned spellings and nothing else", () => { "Task::init(&raw mut *self)", "Task::init(core::ptr::addr_of_mut!(*self))", "jsc::ConcurrentTask::create_from::(std::ptr::from_mut(self))", + "let task = ConcurrentTaskItem::create_from(std::ptr::from_mut(self));", + "bun_event_loop::ConcurrentTask::ConcurrentTask::from_callback(self, on_done)", "let p = self as *mut Self;\nlet ct = ConcurrentTask::create_from(p);", "let p: *mut Self = std::ptr::from_mut(self);\nlet ct = bun_jsc::ConcurrentTask::create_from(p);", "let p = core::ptr::from_ref(self).cast_mut();\nlet ct = ConcurrentTask::create_from(p);", @@ -224,6 +261,11 @@ test("the patterns match the banned spellings and nothing else", () => { "let ct = ConcurrentTask::create(Task::init(this));", "ConcurrentTask::create_from(task.as_ptr())", "ConcurrentTask::from_callback(this, FetchTasklet::resume_request_data_stream)", + "(*this).concurrent_task.from(this, AutoDeinit::ManualDeinit)", + "EventLoopTask::Js(ct) => ct.from(this, AutoDeinit::ManualDeinit),", + // `From::from` and the like are path calls, not the intrusive method. + "let s = String::from(self);", + "let v = Vec::from(self.as_slice());", // Something the receiver owns or points at, or a helper's pointer, is // out of scope (see the header). "ConcurrentTask::create(Task::init(self.as_ctx_ptr()))", @@ -231,8 +273,10 @@ test("the patterns match the banned spellings and nothing else", () => { "Task::init(&raw mut *self.inner)", "Task::init(self.task)", "ConcurrentTask::from_callback(std::ptr::from_mut(load), on_load_from_js_loop_raw)", - // A constructor whose name merely ends in `Task`, and a method call. + // A constructor whose name merely ends in `Task`, a `create_from` that is + // not a task's, and a method call. "NapiFinalizerTask::init(self).schedule();", + "let headers = FetchHeaders::create_from(self);", "state.create_from(index, from, into);", // Producing the pointer is fine when it is not what gets posted. "let this = std::ptr::from_mut::(self);\nregister(this);", From c265bf9def7c9f65af932dd6ff57f1415027c75a Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 12 Aug 2026 13:28:26 +0000 Subject: [PATCH 3/4] bundler: shorten the hand-back contract comments --- src/bundler/BundleThread.rs | 58 ++++++++------------ src/runtime/api/js_bundle_completion_task.rs | 6 +- 2 files changed, 24 insertions(+), 40 deletions(-) diff --git a/src/bundler/BundleThread.rs b/src/bundler/BundleThread.rs index 3563ea273dc9..0884eb8ac80a 100644 --- a/src/bundler/BundleThread.rs +++ b/src/bundler/BundleThread.rs @@ -69,17 +69,14 @@ pub trait CompletionStruct: Node + Send + 'static { /// while it was still queued ([`free_released_unstarted`] then frees it). fn try_start(&mut self) -> bool; fn free_released_unstarted(this: *mut Self); - /// Hands the finished build (result and log already stored) back to its - /// owner, which may free `*this` the moment it is posted: this is the - /// bundle thread's last touch of it. It takes the pointer rather than - /// `&mut self` because a reference argument stays protected until the - /// call returns, and freeing protected memory is UB under both aliasing - /// models; for the same reason the caller must not hold a reference to - /// `*this` across the call either. + /// Hands the build (result and log stored) back to its owner, which may + /// free `*this` as soon as it is posted. Raw pointer, not `&mut self`: no + /// reference to `*this`, here or in the caller, may be live when that + /// happens. /// /// # Safety - /// `this` is a build that `try_start` accepted and the bundle thread still - /// owns; nothing on this thread touches `*this` afterwards. + /// `this` is a started build the bundle thread owns; nothing on this + /// thread touches `*this` afterwards. unsafe fn complete_on_bundle_thread(this: *mut Self); fn set_result(&mut self, result: BundleV2Result); fn set_log(&mut self, log: bun_ast::Log); @@ -249,11 +246,8 @@ impl BundleThread { // No `catch_unwind` — there is nothing to catch. // // SAFETY: started ⇒ `*completion` is ours until it is handed - // back, and handing it back is the last thing either path does - // with it. The owner may free it as soon as it is posted, so it - // stays a raw pointer: `set_result`'s reborrow ends at the `;` - // and no reference to it is live across either hand-back (see - // `complete_on_bundle_thread`). + // back, which is the last thing either path does with it; the + // reborrow for `set_result` ends before that. unsafe { if let Err(err) = Self::generate_in_new_thread(completion, generation) { (*completion).set_result(BundleV2Result::Err(err)); @@ -280,13 +274,9 @@ impl BundleThread { /// This is called from `Bun.build` in JavaScript. /// - /// On `Ok` the build has been handed back (`complete_on_bundle_thread`) - /// and `*completion` may already be gone; on `Err` it is still ours and - /// the caller stores the error and hands it back. - /// /// # Safety - /// `completion` is a build that `try_start` accepted and the bundle thread - /// still owns. + /// `completion` is a started build the bundle thread owns. On `Ok` it has + /// been handed back (and may be gone); on `Err` the caller hands it back. unsafe fn generate_in_new_thread( completion: *mut C, generation: bun_core::Generation, @@ -301,18 +291,16 @@ impl BundleThread { // Allocate + configure folded — see `create_and_configure_transpiler` doc. // - // SAFETY: fn contract. `*completion` is reborrowed for this one call; - // the returned `&'a mut Transpiler` borrows `bump`, not the task. + // SAFETY: fn contract; the reborrow ends with the call (the result + // borrows `bump`). let transpiler = unsafe { (*completion).create_and_configure_transpiler(bump) }?; transpiler.resolver.generation = generation; - // Construction + run delegated — see `init_and_run` doc. Keep a raw - // ptr to `transpiler`: `init_and_run` consumes the `&'a mut`, and the - // log is read (and the struct dropped) through it afterwards. + // Construction + run delegated — see `init_and_run` doc. It consumes + // the `&'a mut`; the log read and the drop below go through this. let transpiler_ptr: *mut Transpiler<'_> = transpiler; - // SAFETY: fn contract (reborrowed for this one call, as above); - // `transpiler` lives in `bump` for the duration of `heap`. + // SAFETY: fn contract; `transpiler` lives in `bump` until `heap` drops. let run = unsafe { (*completion).init_and_run( &mut *transpiler_ptr, @@ -329,19 +317,17 @@ impl BundleThread { // `deinitWithoutFreeingArena` + wait-group drain live inside `init_and_run` // (it owns `this`). let mut out_log = bun_ast::Log::init(); - // SAFETY: `transpiler.log` is the `*mut Log` that - // `create_and_configure_transpiler` installed (the one impl points it - // into the task, which is still ours here). Raw deref so the `&'a mut - // Transpiler` consumed by `init_and_run` above is not reborrowed. + // SAFETY: `transpiler.log` was installed by `create_and_configure_transpiler` + // and is live while the build is ours; raw because `init_and_run` + // consumed the `&'a mut`. let _ = unsafe { (*(*transpiler_ptr).log).append_to_with_recycled(&mut out_log, true) }; // logger OOM-only - // SAFETY: fn contract; reborrowed for this one call. + // SAFETY: fn contract; the reborrow ends with the call. unsafe { (*completion).set_log(out_log) }; if run.is_ok() { - // SAFETY: fn contract; the result and log are stored, the - // reborrows above have ended, and nothing below touches - // `*completion` (or, through `transpiler.log`, the task's log): the - // teardown drops only memory this thread owns. + // SAFETY: result and log are stored and no reborrow is live; + // nothing below touches `*completion` (the teardown drops only this + // thread's memory). unsafe { C::complete_on_bundle_thread(completion) }; } diff --git a/src/runtime/api/js_bundle_completion_task.rs b/src/runtime/api/js_bundle_completion_task.rs index f5d61f76e8a0..b7ead71172ee 100644 --- a/src/runtime/api/js_bundle_completion_task.rs +++ b/src/runtime/api/js_bundle_completion_task.rs @@ -1117,10 +1117,8 @@ impl CompletionStruct for JSBundleCompletionTask { unsafe fn complete_on_bundle_thread(this: *mut Self) { // The bundle thread's last touch of this task and of the VM's memory: // hand it back (always queued — the VM waits for it) and stop counting. - // The post carries the creation ref, which `on_complete_anytask` - // releases, so the JS thread may free `*this` as soon as the post - // lands: everything needed afterwards is taken out first, through - // accesses that end at their `;`. + // The post carries the creation ref (`on_complete_anytask` releases it), + // so the JS thread may free `*this` once it lands: take the handle out first. // // SAFETY: trait contract — `this` is the started build, still ours. let handle = unsafe { From 2ba08882953b433d6ebebbeacc740d9bbe158f1d Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 12 Aug 2026 13:44:29 +0000 Subject: [PATCH 4/4] source-lints: name the PRs converting each self-receiver-publish exception The embedded-task .from() spelling is covered by the sibling lint in #37750, so this one goes back to the heap-task constructors only. --- .../self-receiver-publish.test.ts | 72 ++++++++----------- 1 file changed, 31 insertions(+), 41 deletions(-) diff --git a/test/internal/source-lints/self-receiver-publish.test.ts b/test/internal/source-lints/self-receiver-publish.test.ts index 8cdcb274c31a..0255349c8fe8 100644 --- a/test/internal/source-lints/self-receiver-publish.test.ts +++ b/test/internal/source-lints/self-receiver-publish.test.ts @@ -13,8 +13,10 @@ import { globAllSources } from "../../../scripts/glob-sources.ts"; // let this = std::ptr::from_mut(self); ... Task::init(this) // let p: *mut Self = self; ... ConcurrentTask::create_from(p) // -// as the argument of `Task::init(..)`, `ConcurrentTask::create_from(..)`, -// `ConcurrentTask::from_callback(..)` or the intrusive `.from(..)` is banned. +// as the argument of `Task::init(..)`, `ConcurrentTask::create_from(..)` or +// `ConcurrentTask::from_callback(..)`, the heap-task constructors, is banned. +// (Filling an embedded task with it, `.from(..)`, is the sibling lint +// self-receiver-intrusive-post.test.ts from #37750.) // // Two things are wrong with it, and which one bites depends on who drains the // queue: @@ -54,23 +56,22 @@ import { globAllSources } from "../../../scripts/glob-sources.ts"; // `complete_on_bundle_thread` in src/runtime/api/js_bundle_completion_task.rs // (and its caller in src/bundler/BundleThread.rs, which keeps the pointer // raw), `async_job_run` in src/runtime/node/node_zlib_binding.rs, `post_job` -// in src/jsc/VmHandle.rs, the `(*this).concurrent_task.from(this, ..)` posts -// in src/runtime/webcore/s3/. +// in src/jsc/VmHandle.rs. // -// Scope: the spellings above, with `self` as the receiver and those four +// Scope: the spellings above, with `self` as the receiver and those three // callees. The same hand-over spelled some other way is outside the regex and -// is the same bug; the instances known at the time of writing, each tracked -// for its own conversion, are `napi_async_work::run` / `post_to_js_thread` in +// is the same bug; the instances known at the time of writing, and where each +// is being converted, are `napi_async_work::run` / `post_to_js_thread` in // src/runtime/napi/napi_body.rs (posts a `*mut Self` parameter the caller made -// from its `&mut self`), `FetchTasklet::deref_from_thread` in +// from its `&mut self`; #37750), `TranspilerJob::dispatch_to_main_thread` in +// src/jsc/RuntimeTranspilerStore.rs (pushes `NonNull::from(&mut *self)` onto +// its own queue; #37778), `FetchTasklet::deref_from_thread` in // src/runtime/webcore/fetch/FetchTasklet.rs (posts `this`, but through -// `post(&self)` and the handle field inside the object it is freeing), +// `post(&self)` and the handle field inside the object it is freeing) and // `StatWatcher::post_to_js_thread(&self)` in src/runtime/node/ -// node_fs_stat_watcher.rs (a helper's pointer, `self.as_ctx_ptr()`), and -// `TranspilerJob::dispatch_to_main_thread(&mut self)` in -// src/jsc/RuntimeTranspilerStore.rs (pushes `NonNull::from(&mut *self)` onto -// its own queue). Convert anything of that shape on sight; the ratchet below -// only tracks what the regex can see. Siblings: self-receiver-reclaim.test.ts, +// node_fs_stat_watcher.rs (a helper's pointer, `self.as_ctx_ptr()`), the last +// two tracked. Convert anything of that shape on sight; the ratchet below only +// tracks what the regex can see. Siblings: self-receiver-reclaim.test.ts, // fn-long-mut-reborrow.test.ts, frozen-nonnull-reborrow.test.ts. const root = path.resolve(import.meta.dir, "..", "..", ".."); @@ -91,14 +92,12 @@ const tracked: Set | null = (() => { // `Task::init(`, `ConcurrentTask::create_from(`, `ConcurrentTask::from_callback(`, // however the path in front is spelled (`jsc::`, `bun_event_loop::ConcurrentTask::`, -// the `ConcurrentTaskItem` alias), optionally turbofished, plus the intrusive -// `.from(` of an embedded `ConcurrentTask` / `AnyTask` (the tree's only -// method-call `.from(`). The `\b` before `Task` keeps `NapiFinalizerTask:: -// init(self)` (a constructor that copies out of `self`) out, and pinning the -// other two to a `ConcurrentTask*` path keeps unrelated `create_from` -// constructors out. `\s*` after the paren so a rustfmt-wrapped argument still -// matches. -const POST = String.raw`(?:\bTask::init|\bConcurrentTask\w*::(?:create_from|from_callback)|\.from)(?:::<[^>]*>)?\(\s*`; +// the `ConcurrentTaskItem` alias), optionally turbofished. The `\b` before +// `Task` keeps `NapiFinalizerTask::init(self)` (a constructor that copies out +// of `self`) out, and pinning the other two to a `ConcurrentTask*` path keeps +// unrelated `create_from` constructors out. `\s*` after the paren so a +// rustfmt-wrapped argument still matches. +const POST = String.raw`\b(?:Task::init|ConcurrentTask\w*::(?:create_from|from_callback))(?:::<[^>]*>)?\(\s*`; // The ways of spelling "`self`, as a raw pointer" as the first argument. Each // is anchored at its front only, so a trailing `.cast_mut()` / `.as_ptr()` @@ -167,15 +166,14 @@ function lineOf(text: string, offset: number): number { // Documented, ratcheted exceptions: files allowed to keep exactly N of the // shape. Each has been read and is the bug described above, not a false -// positive; each is listed because its conversion is a change of its own, -// tracked separately, not because it is safe. Lower an entry when you convert -// one; do not add entries. +// positive; each is listed because its conversion is a change of its own (the +// PR named on it), not because it is safe. Lower an entry when its conversion +// lands; do not add entries. const ALLOW: Record = { // `Resolve::dispatch` / `Load::dispatch` (`&mut self`) post the arena-owned // plugin request to the JS thread, which writes its `value` while // `dispatch` is still returning (the arena, not the consumer, frees it). - // Conversion: `dispatch(this: *mut Self)` with the pointer the callers - // already hold. + // #37732 converts them; delete this entry when it lands. "src/bundler/bundle_v2.rs": 2, // `DeferredBatchTask::schedule` (`&mut self`) posts a task embedded in its // `BundleV2`, whose consumer reaches the surrounding `BundleV2` through it; @@ -186,11 +184,10 @@ const ALLOW: Record = { // `ThreadSafeFunction::maybe_queue_finalizer` is the same-thread case: it // sets `closing` and posts `self_ptr` to its own loop, `on_dispatch` then // CASes `dispatch_state` through the real pointer on its way out, and the - // drained task is what `destroy`s the TSFN, through the dead one. - // `schedule_dispatch` is the cross-thread case (addon threads post, the JS - // thread writes the atomics at once). Both are reached through `&mut self` - // callers (`dispatch_one`; `call` / `release_locked`) that have to be - // converted with them. + // drained task is what `destroy`s the TSFN, through the dead one (#37762, + // with its `dispatch_one` caller). `schedule_dispatch` is the cross-thread + // case: addon threads post and the JS thread writes the atomics at once + // (#37741, with its `call` / `release_locked` callers). One each. "src/runtime/napi/napi_body.rs": 2, }; @@ -232,10 +229,6 @@ test("the patterns match the banned spellings and nothing else", () => { "bun_event_loop::ConcurrentTask::ConcurrentTask::create(\n bun_event_loop::Task::init(std::ptr::from_mut::(self)),\n);", "let self_ptr: *mut Self = self;\nif done {\n return;\n}\nlet ct = ConcurrentTask::create_from(self_ptr);", "let self_ptr: *mut Self = self;\nloop_.enqueue_task(Task::init(self_ptr));", - // The intrusive form. - "let ct = self.concurrent_task.from(std::ptr::from_mut(self), AutoDeinit::ManualDeinit);", - "let self_ptr: *mut Self = self;\nlet ct = self.concurrent_task.from(self_ptr, AutoDeinit::ManualDeinit);", - "at.from(self, Self::run_from_main_thread_mini)", // Other spellings of the pointer; a same-loop post counts too (see the // header). "loop_.enqueue_task(Task::init(self));", @@ -261,11 +254,8 @@ test("the patterns match the banned spellings and nothing else", () => { "let ct = ConcurrentTask::create(Task::init(this));", "ConcurrentTask::create_from(task.as_ptr())", "ConcurrentTask::from_callback(this, FetchTasklet::resume_request_data_stream)", - "(*this).concurrent_task.from(this, AutoDeinit::ManualDeinit)", - "EventLoopTask::Js(ct) => ct.from(this, AutoDeinit::ManualDeinit),", - // `From::from` and the like are path calls, not the intrusive method. - "let s = String::from(self);", - "let v = Vec::from(self.as_slice());", + // The embedded-task form is the sibling lint's (see the header). + "let ct = self.concurrent_task.from(self_ptr, AutoDeinit::ManualDeinit);", // Something the receiver owns or points at, or a helper's pointer, is // out of scope (see the header). "ConcurrentTask::create(Task::init(self.as_ctx_ptr()))",