fix(worker): delay Blob URL revocation to ensure webkit compatibility #19565

Antraxmin · 2025-03-03T21:15:03Z

Description

This PR fixes an issue where WebKit-based browsers (Playwright WebKit & Safari) failed to create workers using Blob URLs due to premature revocation of URL.createObjectURL.
The fix ensures that revokeObjectURL is called only after the worker has successfully initialized, preventing WebKit from invalidating the Blob URL too early.
Additionally, for SharedWorkers, the worker is now created using a data URL instead of a Blob URL to prevent unintended multiple instances.

Issue Reference

Related Issue: #19238

Fix Details

Delayed revokeObjectURL(objURL): Now executed inside a worker.addEventListener("message", ...) instead of immediately.
SharedWorker Handling: Uses data:text/javascript;charset=utf-8, ... instead of a Blob URL to ensure proper behavior.
WebKit Compatibility: Prevents workers from breaking due to revoked URLs before execution.

Testing

Playwright WebKit Test Results
Tests executed successfully using Playwright WebKit:

$ pnpm exec playwright test --project=webkit

Running 1 test using 1 worker

  ✓  1 [webkit] › index.spec.js:9:1 › renders message (515ms)

  1 passed (1.7s)

sapphi-red · 2025-03-04T01:46:17Z

packages/vite/src/node/plugins/worker.ts

-              worker.addEventListener("error", () => {
+              worker.addEventListener("message", () => {


We should call revokeObjectURL if an error happened regardless of whether a message has been received because an error may happen before a message is received (or the user may not send any messages).
We need to append revokeObjectURL call to the script so it is called even if the user didn't send any messages (for ESM it's done above in the declaration of blob variable).

Antraxmin · 2025-03-04T03:13:50Z

@sapphi-red Thanks for the feedback! To ensure revokeObjectURL is always called, I plan to:

Call revokeObjectURL on worker creation failure.
Retain revokeObjectURL in the message event.
Append self.addEventListener("error", ...) for classic workers to handle early errors.
Ensure revokeObjectURL is appended to the script itself for ESM workers,
so it executes even if no messages are sent.

Would this approach work? Let me know if you have any suggestions! 🙌

sapphi-red · 2025-03-04T04:15:24Z

I think that approach would work.

Retain revokeObjectURL in the message event.

I think the message event handler can be removed. If the revokeObjectURL call is appended to the script, then the URL is already revoked.

This reverts commit 2a4e8da.

Antraxmin · 2025-03-04T11:30:26Z

@sapphi-red
I've implemented the improvements you suggested:

Ensured revokeObjectURL is always called by appending it to the script.
Removed redundant revokeObjectURL call from the message event.
Improved error handling for better memory management.

However, I'm facing a linting issue because URL.revokeObjectURL is considered an experimental feature in Node.js, and ESLint blocks it:

Error: The 'URL.revokeObjectURL' is still an experimental feature
The configured version range is '^18.0.0 || ^20.0.0 || >=22.0.0'

Would it be better to conditionally check the Node.js version before calling it?
Or should we handle this in a different way to ensure compatibility across environments?

Antraxmin · 2025-03-04T13:06:36Z

Hi @sapphi-red,
I've implemented the changes you suggested for URL.revokeObjectURL:

Ensured revokeObjectURL is always called at the right moment.
Removed redundant revokeObjectURL calls.
Improved error handling to prevent memory leaks.

However, while reviewing the CI failures, I noticed that the failing tests are not related to this change.
Specifically, server-source-maps.spec.ts is failing due to unexpected stack trace outputs:

The test expects source maps to map errors to their original module (fixtures/has-error.js).
Instead, the stack trace points to server-source-maps.spec.ts, which suggests that source maps are not applied as expected.
The test also fails in deep stacktrace, where error locations appear inside Vitest’s internal runner instead of the expected module.

This issue seems independent of this PR and affects CI across multiple Node.js versions.

So, I wanted to check:

Should we separate this PR? Since the revokeObjectURL changes are unrelated to the failing tests, I could submit it separately to get it merged without waiting for test fixes.
How should we handle the CI failures? Would it be best to investigate server-source-maps.spec.ts separately, or are there any known issues with source maps handling in Vitest?

Let me know what you think! Thanks as always for your guidance.

sapphi-red · 2025-03-05T05:12:55Z

However, I'm facing a linting issue because URL.revokeObjectURL is considered an experimental feature in Node.js, and ESLint blocks it:

It's because you are calling URL.revokeObjectURL while building the bundle. It has to be called when the bundle is executed.

I noticed that the failing tests are not related to this change.

I don't see those failures in the CI. The worker related tests are failing due to incorrect syntax.
https://github.com/vitejs/vite/actions/runs/13652577528/job/38164336881?pr=19565#step:13:208

- Ensures proper cleanup by appending revokeObjectURL to the worker script - Fixes WebKit compatibility issues by managing Blob URL lifecycle - Maintains SharedWorker functionality using data URLs - Moves URL.revokeObjectURL call to runtime execution

Antraxmin · 2025-03-05T08:52:19Z

Hi @sapphi-red, just an update! All CI checks have now passed successfully.

c3c18b6

I’ve fully implemented your feedback, including:

Ensuring URL.revokeObjectURL is only called at runtime, not during the build process.
Appending revokeObjectURL to the worker script itself, so it gets executed even if no messages are sent.
Handling Worker creation errors by properly revoking the Blob URL when necessary.

Let me know if there's anything else you'd like me to adjust. Thanks again for your feedback.

sapphi-red · 2025-03-05T10:35:10Z

packages/vite/src/node/plugins/worker.ts

              const worker = new ${workerConstructor}(objURL, ${workerTypeOption});
-              worker.addEventListener("error", () => {
-                (self.URL || self.webkitURL).revokeObjectURL(objURL);
-              });
-              return worker;
+              // Append cleanup code to the worker script to ensure revokeObjectURL is called
+              const originalCode = jsContent;
+              const cleanupCode = ';(function(){const cleanup=function(){(self.URL||self.webkitURL).revokeObjectURL("'+objURL+'")};self.addEventListener("error",cleanup);})();';
+              const finalCode = originalCode + cleanupCode;
+              const cleanupBlob = new Blob([finalCode], { type: "text/javascript;charset=utf-8" });
+              const cleanupURL = (self.URL || self.webkitURL).createObjectURL(cleanupBlob);
+              return new ${workerConstructor}(cleanupURL, ${workerTypeOption});


I don't think we need to create two Workers.

fix: ensure webKit workers retain blob urls correctly

17ae83a

Antraxmin changed the title ~~fix(worker): Delay Blob URL revocation to ensure WebKit compatibility~~ fix(worker): delay Blob URL revocation to ensure webkit compatibility Mar 3, 2025

sapphi-red added p3-minor-bug An edge case that only affects very specific usage (priority) feat: web workers labels Mar 4, 2025

sapphi-red requested changes Mar 4, 2025

View reviewed changes

Antraxmin added 5 commits March 4, 2025 18:42

fix(worker): ensure revokeObjectURL is always called properly

363fe66

fix(worker): safely handle URL.revokeObjectURL

2a4e8da

Revert "fix(worker): safely handle URL.revokeObjectURL"

7b9ad98

This reverts commit 2a4e8da.

fix(worker): safely handle URL.revokeObjectURL

be5aaa4

fix(worker): ensure proper cleanup of revoked URLs

4e64e5d

sapphi-red requested changes Mar 5, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(worker): delay Blob URL revocation to ensure webkit compatibility #19565

fix(worker): delay Blob URL revocation to ensure webkit compatibility #19565

Antraxmin commented Mar 3, 2025

sapphi-red Mar 4, 2025

Antraxmin commented Mar 4, 2025

sapphi-red commented Mar 4, 2025

Antraxmin commented Mar 4, 2025

Antraxmin commented Mar 4, 2025

sapphi-red commented Mar 5, 2025

Antraxmin commented Mar 5, 2025 •

edited

Loading

sapphi-red Mar 5, 2025

		worker.addEventListener("error", () => {
		worker.addEventListener("message", () => {

fix(worker): delay Blob URL revocation to ensure webkit compatibility #19565

Are you sure you want to change the base?

fix(worker): delay Blob URL revocation to ensure webkit compatibility #19565

Conversation

Antraxmin commented Mar 3, 2025

Description

Issue Reference

Fix Details

Testing

sapphi-red Mar 4, 2025

Choose a reason for hiding this comment

Antraxmin commented Mar 4, 2025

sapphi-red commented Mar 4, 2025

Antraxmin commented Mar 4, 2025

Antraxmin commented Mar 4, 2025

sapphi-red commented Mar 5, 2025

Antraxmin commented Mar 5, 2025 • edited Loading

sapphi-red Mar 5, 2025

Choose a reason for hiding this comment

Antraxmin commented Mar 5, 2025 •

edited

Loading