-
Notifications
You must be signed in to change notification settings - Fork 5k
bundler: keep the scan counter balanced when an onLoad plugin does not await defer() #37731
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
robobun
wants to merge
6
commits into
main
Choose a base branch
from
farm/14792fcd/bundler-defer-noawait
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
e3571ed
bundler: keep the scan counter balanced when an onLoad plugin does no…
robobun 7b9bd4e
test: fail the cancellation fixture fast when the worker errors or th…
robobun 8ac83ee
bundler: trim the defer() bookkeeping comments
robobun c572221
ci: retrigger
robobun 757026e
bundler: settle unawaited defer() promises at build completion and ke…
robobun 365aff8
test: make the unawaited defer() dev server case deterministic and wi…
robobun File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,84 +1,51 @@ | ||
| //! This task is run once all parse and resolve tasks have been complete | ||
| //! and we have deferred onLoad plugins that we need to resume. | ||
| //! Posted to the plugins' JS thread when the scan has nothing left to do but | ||
| //! the onLoad callbacks that called `.defer()`: the runtime's dispatch arm | ||
| //! resolves their promises (`JSBundlerPlugin__drainDeferred`) and frees this. | ||
| //! | ||
| //! It enqueues a task to be run on the JS thread which resolves the promise | ||
| //! for every onLoad callback which called `.defer()`. | ||
| //! It carries only the plugin handle. A plugin that answers without awaiting | ||
| //! its `.defer()` promise lets the pass finish, and free its `BundleV2`, while | ||
| //! this task is still queued, so nothing here may point back into the pass. | ||
| //! The handle itself outlives the task: `Bun.build` destroys it from the | ||
| //! completion task, which is posted to the same queue after this, and bake's | ||
| //! plugins live as long as the dev server. | ||
|
robobun marked this conversation as resolved.
|
||
|
|
||
| use core::ptr::NonNull; | ||
|
|
||
| use crate::BundleV2; | ||
| // Task is `(tag: u8, ptr: *mut ())` owned by bun_event_loop; | ||
| // runtime owns the match-loop. See PORTING.md §Dispatch. | ||
| use crate::bundle_v2::JSBundlerPlugin; | ||
| use bun_event_loop::ConcurrentTask::ConcurrentTask; | ||
| use bun_event_loop::{Task, task_tag}; | ||
|
|
||
| #[derive(Default)] | ||
| pub struct DeferredBatchTask { | ||
| // Debug-only flag; zero-sized in release. | ||
| #[cfg(debug_assertions)] | ||
| running: bool, | ||
| plugins: NonNull<JSBundlerPlugin>, | ||
| } | ||
|
|
||
| impl bun_event_loop::Taskable for DeferredBatchTask { | ||
| const TAG: bun_event_loop::TaskTag = task_tag::BundleV2DeferredBatchTask; | ||
| /// Embedded in its `BundleV2`, which outlives the queue entry and owns | ||
| /// everything the drain would have touched; nothing to free. | ||
| unsafe fn release_unrun(_: *mut Self) {} | ||
| unsafe fn release_unrun(this: *mut Self) { | ||
| // SAFETY: fn contract — `this` is the box `schedule` queued. | ||
| unsafe { bun_core::heap::destroy(this) }; | ||
| } | ||
| } | ||
|
|
||
| impl DeferredBatchTask { | ||
| pub(crate) fn init(&mut self) { | ||
| // Kept as `&mut self` (not `-> Self`) — this struct is embedded | ||
| // by value in BundleV2 (recovered via container_of in `get_bundle_v2`), so | ||
| // it is reset in place, never separately constructed. | ||
| #[cfg(debug_assertions)] | ||
| debug_assert!(!self.running); | ||
| // No Drop / no owned fields — pure reset. | ||
| let _ = core::mem::take(self); | ||
| } | ||
|
|
||
| pub(crate) fn get_bundle_v2(&mut self) -> &mut BundleV2<'static> { | ||
| // SAFETY: `self` is always the `drain_defer_task` field of a live `BundleV2`; | ||
| // this struct is never instantiated standalone. Lifetime erased to 'static; | ||
| // callers must not outlive the owning bundle. | ||
| unsafe { | ||
| &mut *bun_core::from_field_ptr!( | ||
| BundleV2<'static>, | ||
| drain_defer_task, | ||
| std::ptr::from_mut::<Self>(self) | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| pub(crate) fn schedule(&mut self) { | ||
| #[cfg(debug_assertions)] | ||
| { | ||
| debug_assert!(!self.running); | ||
| self.running = false; | ||
| } | ||
| let task = ConcurrentTask::create(Task::init(std::ptr::from_mut::<Self>(self))); | ||
|
|
||
| self.get_bundle_v2().enqueue_on_js_loop_for_plugins(task); | ||
| } | ||
|
|
||
| pub fn run_on_js_thread(&mut self) { | ||
| // `deinit` only resets | ||
| // the debug `running` flag; nothing follows `drain_deferred`, so | ||
| // resetting the flag afterwards covers both paths. | ||
| { | ||
| let bv2 = self.get_bundle_v2(); | ||
| let rejected = bv2.completion.map(|c| c.result_is_err()).unwrap_or(false); | ||
| // The void result is discarded — see | ||
| // `Plugin::drain_deferred` for the exception-scope note. | ||
| bv2.plugins_mut().expect("plugins").drain_deferred(rejected); | ||
| /// Bundle thread (the loop that owns `bv2`). | ||
| pub(crate) fn schedule(bv2: &mut BundleV2) { | ||
| let plugins = bv2 | ||
| .plugins | ||
| .expect("a load deferred, so the pass has plugins"); | ||
| let this = bun_core::heap::into_raw(Box::new(Self { plugins })); | ||
| let task = ConcurrentTask::create(Task::init(this)); | ||
| if !bv2.enqueue_on_js_loop_for_plugins(task) { | ||
| // SAFETY: refused ⇒ the queue never took `this`; nothing else points at it. | ||
| unsafe { bun_core::heap::destroy(this) }; | ||
| } | ||
| self.deinit(); | ||
| } | ||
|
|
||
| // Not `impl Drop` — this struct is an intrusive field of `BundleV2` | ||
| // and `deinit` is a debug-flag reset, not resource teardown. | ||
| fn deinit(&mut self) { | ||
| #[cfg(debug_assertions)] | ||
| { | ||
| self.running = false; | ||
| } | ||
| /// JS thread, from the dispatch arm that owns the box. | ||
| pub fn plugins(&self) -> &JSBundlerPlugin { | ||
| // SAFETY: see the module doc — the handle is destroyed only after this | ||
| // task has run or been released. | ||
| unsafe { self.plugins.as_ref() } | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.