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

Segmentation Fault when importing an image file with query parameters #9521

Closed
patriksimms opened this issue Mar 20, 2024 · 2 comments · Fixed by #9530
Closed

Segmentation Fault when importing an image file with query parameters #9521

patriksimms opened this issue Mar 20, 2024 · 2 comments · Fixed by #9530
Labels
bug Something isn't working crash An issue that could cause a crash

Comments

@patriksimms
Copy link

What version of Bun is running?

1.0.33

What platform is your computer?

Darwin 23.4.0 arm64 arm

What steps can reproduce the bug?

Was not able to create a minimal reproduciable repo for this so I will try to note everything relevant here

  • Have an react component as tsx file (e.g. someComponent.tsx)
  • import an image via an JS import and query parameters
    e.g. import clippy_webp from '../../../assets/img/Clippy.webp?format=webp&h=25&=25'
  • return an jsx element which includes the js import as a src attribute
    e.g. return <img src={clippy_webp} alt="Clippy" />
  • run a bun test with a test that imports the file

What is the expected behavior?

the test succeeds every time (no segmentation fault)

What do you see instead?

when having one import with query parameters, in 1/10 of the cases I get the following error:
[1] 6830 segmentation fault bun test

When adding more imports with query paremeters, the chance of failure increases until at ~10 imports it pretty much always fails.

Additional information

The whole query parameter for image imports is used by this

I tried really hard to reproduce the issue in a minimal repo but failed. What I tried:

  • just having an image import with query parameters
  • trying out different query parameters
  • trying out different image types
  • referencing the import in function returns
  • referencing the import in jsx returns

I also took a look at the bun source code and identify the issue but failed to write a meaningful test to verify my assumptions (also not a zig dev so dont know about string specifics). But I assume it is something with this line copying to much/less
https://github.com/oven-sh/bun/blob/main/src/bun.js/module_loader.zig#L2084

@patriksimms patriksimms added the bug Something isn't working label Mar 20, 2024
@Jarred-Sumner
Copy link
Collaborator

Minimal reproduction:

var c = 0;
async function reloadImport() {
  (await import("./empty.txt?" + c++)).default;
}

for (let i = 0; i < 10000; i++) {
  await reloadImport();
}

Jarred-Sumner added a commit that referenced this issue Mar 20, 2024
@patriksimms
Copy link
Author

Crazy fast🚀 Thanks for taking a look!

@Electroid Electroid added the crash An issue that could cause a crash label Mar 20, 2024
Jarred-Sumner added a commit that referenced this issue Mar 21, 2024
* Fixes #9521

* Another test

* Update load-file-loader-a-lot.test.ts

* Update module_loader.zig

* comments

* Rename variable

* Update module_loader.zig

* Update exports.zig

* small cleanup

* bundows

---------

Co-authored-by: Jarred Sumner <709451+Jarred-Sumner@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working crash An issue that could cause a crash
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants