Skip to content
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

Fix macros which call Bun.file operations causing build commands to hang #15991

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

brainkim
Copy link
Contributor

@brainkim brainkim commented Dec 26, 2024

What does this PR do?

  • Documentation or TypeScript types (it's okay to leave the rest blank in this case)
  • Code changes

Users reported in #7611 and #14104 that calling Bun.file().text() in a macro would cause builds to hang. This bug was introduced in v1.0.16, likely when some changes related to how Bun handles multithreading occurred.

I went through a minimal reproduction with a debugger and determined that work would get stuck in ThreadPool threads because the ThreadPool was initialized twice. The double instantiation of ThreadPool, which is assumed to be a global singleton, caused issues in macros because file I/O would get scheduled on a thread, the work would be pushed to the thread local queue/buffer, and then the thread would wait for the promise to resolve, causing a dead lock. There is code which allows for “work stealing” but threads initialized from different thread pools do not steal work each other.

The least invasive fix I found was to consistently call WorkPool.get() in the bundle_v2’s initialization code.

Fixes #7611 and fixes #14104.
Happy holidays!

How did you verify your code works?

  • I included a test for the new code, or existing tests cover it
  • I ran my tests locally and they pass (bun-debug test test-file-name.test)

@brainkim brainkim changed the title Fix macros which call Bun.file operations from causing build commands to hang Fix macros which call Bun.file operations causing build commands to hang Dec 26, 2024
@brainkim brainkim marked this pull request as draft December 26, 2024 13:56
@brainkim
Copy link
Contributor Author

I guess things are never this easy...

@Jarred-Sumner
Copy link
Collaborator

My guess is that by not using the bundler’s threadpool, the threads never call their init function

it sounds like we should have a global variable for “threadpool ID” and only use the global one in that case maybe?

@brainkim
Copy link
Contributor Author

brainkim commented Dec 28, 2024

I tried building on ubuntu and the failing test passes locally. I also tested it by hand.

Screenshot 2024-12-28 at 12 16 20 AM

Is it possible that the CI is running the tests against a stale build? I’m not sure how this failure could be platform dependent. Perhaps it is due to the number of threads available?

Running the build process in a container takes a painfully long time, wishing for cross-compilation here.

Screenshot 2024-12-27 at 7 44 59 PM

@brainkim brainkim marked this pull request as ready for review December 28, 2024 01:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bun build stucks with async reading file in macro Build macros hang on Bun.file(...).text()
2 participants