[pull] main from oven-sh:main#248
Merged
Merged
Conversation
### What does this PR do? <!-- **Please explain what your changes do** --> This PR should fix #14219 and implement `WebAssembly.instantiateStreaming()` and `WebAssembly.compileStreaming()`. This is a mixture of WebKit's implementation (using a helper, `handleResponseOnStreamingAction`, also containing a fast-path for blobs) and some of Node.js's validation (error messages) and its builtin-based strategy to consume chunks from streams. `src/bun.js/bindings/GlobalObject.zig` has a helper function (`getBodyStreamOrBytesForWasmStreaming`), called by C++, to validate the response (like [Node.js](https://github.com/nodejs/node/blob/214e4db60ef55f1a59c93336ab956371b0b058ed/lib/internal/wasm_web_api.js) does) and to extract the data from the response, either as a slice/span (if we can get the data synchronously), or as a `ReadableStream` body (if the data is still pending or if it is a file/S3 `Blob`). In C++, `handleResponseOnStreamingAction` is called by `compileStreaming` and `instantiateStreaming` on the `JSC::GlobalObjectMethodTable`, just like in [WebKit](https://github.com/oven-sh/WebKit/blob/97ee3c598ac4226dd1c759739acf94f70cb10ed0/Source/WebCore/bindings/js/JSDOMGlobalObject.cpp#L517). It calls the aforementioned Zig helper for validation and getting the response data. The data is then fed into `JSC::Wasm::StreamingCompiler`. If the data is received as a `ReadableStream`, then we call a JS builtin in `WasmStreaming.ts` to iterate over each chunk of the stream, like [Node.js](https://github.com/nodejs/node/blob/214e4db60ef55f1a59c93336ab956371b0b058ed/lib/internal/wasm_web_api.js#L50-L52) does. The `JSC::Wasm::StreamingCompiler` is passed into JS through a new wrapper object, `WebCore::WasmStreamingCompiler`, like [Node.js](https://github.com/nodejs/node/blob/214e4db60ef55f1a59c93336ab956371b0b058ed/src/node_wasm_web_api.h) does. It has `addBytes`, `finalize`, `error`, and (unused) `cancel` methods to mirror the underlying JSC class. (If there's a simpler way to do this, please let me know...that would be very much appreciated) - [x] Code changes ### How did you verify your code works? <!-- **For code changes, please include automated tests**. Feel free to uncomment the line below --> I wrote automated tests (`test/js/web/fetch/wasm-streaming.test`). <!-- If JavaScript/TypeScript modules or builtins changed: --> - [x] I included a test for the new code, or existing tests cover it - [x] I ran my tests locally and they pass (`bun-debug test test/js/web/fetch/wasm-streaming.test`) <!-- If Zig files changed: --> - [x] I checked the lifetime of memory allocated to verify it's (1) freed and (2) only freed when it should be (NOTE: consumed `AnyBlob` bodies are freed, and all other allocations are in C++ and either GCed or ref-counted) - [x] I included a test for the new code, or an existing test covers it (NOTE: via JS/TS unit test) - [x] JSValue used outside of the stack is either wrapped in a JSC.Strong or is JSValueProtect'ed (NOTE: N/A, JSValue never used outside the stack) - [x] I wrote TypeScript/JavaScript tests and they pass locally (`bun-debug test test/js/web/fetch/wasm-streaming.test`) --------- Co-authored-by: graphite-app[bot] <96075541+graphite-app[bot]@users.noreply.github.com>
…he main thread (#21425) ### What does this PR do? Fixes thread safety issues due to file poll code being not thread safe. <!-- **Please explain what your changes do**, example: --> <!-- This adds a new flag --bail to bun test. When set, it will stop running tests after the first failure. This is useful for CI environments where you want to fail fast. --> ### How did you verify your code works? Added tests for lifecycle scripts. The tests are unlikely to reproduce the bug, but we'll know if it actually fixes the issue if `test/package.json` doesn't show in flaky tests anymore. <!-- **For code changes, please include automated tests**. Feel free to uncomment the line below --> <!-- I wrote automated tests --> <!-- If JavaScript/TypeScript modules or builtins changed: - [ ] 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`) --> <!-- If Zig files changed: - [ ] I checked the lifetime of memory allocated to verify it's (1) freed and (2) only freed when it should be - [ ] I included a test for the new code, or an existing test covers it - [ ] JSValue used outside of the stack is either wrapped in a JSC.Strong or is JSValueProtect'ed - [ ] I wrote TypeScript/JavaScript tests and they pass locally (`bun-debug test test-file-name.test`) --> <!-- If new methods, getters, or setters were added to a publicly exposed class: - [ ] I added TypeScript types for the new methods, getters, or setters --> <!-- If dependencies in tests changed: - [ ] I made sure that specific versions of dependencies are used instead of ranged or tagged versions --> <!-- If a new builtin ESM/CJS module was added: - [ ] I updated Aliases in `module_loader.zig` to include the new module - [ ] I added a test that imports the module - [ ] I added a test that require() the module --> --------- Co-authored-by: taylor.fish <contact@taylor.fish>
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
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
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.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.3)
Can you help keep this open source service alive? 💖 Please sponsor : )