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

Importing a revoked blob URL succeeds if it was imported in a different worker before revoking #12248

Open
andreubotella opened this issue Sep 27, 2021 · 3 comments
Labels
bug Something isn't working correctly web related to Web APIs

Comments

@andreubotella
Copy link
Contributor

Different workers/threads have different module graphs, which means importing the same module will result in the same module object within a same thread, but in different module objects across threads:

// main.js
const module1 = await import("./randomId.js");
console.log("Main thread (#1):", module1.RANDOM_ID);

const module2 = await import("./randomId.js");
console.log("Main thread (#2):", module2.RANDOM_ID);

const worker = new Worker(
    new URL("./worker.js", import.meta.url),
    {type: "module"}
);
// worker.js
const module1 = await import("./randomId.js");
console.log("Worker thread (#1):", module1.RANDOM_ID);

const module2 = await import("./randomId.js");
console.log("Worker thread (#2):", module2.RANDOM_ID);
// randomId.js
export const RANDOM_ID = crypto.randomUUID();
Main thread (#1): 23f163b0-e80e-41d1-8363-cb36b83f96be
Main thread (#2): 23f163b0-e80e-41d1-8363-cb36b83f96be
Worker thread (#1): fd2a439a-6db4-4d84-a96c-d8d05ef4d043
Worker thread (#2): fd2a439a-6db4-4d84-a96c-d8d05ef4d043

This means that within a thread/worker, you can import now-revoked blob URLs if they were imported for the first time before being revoked:

const blob = new Blob([`console.log("Hi from blob!");`], {type: "text/javascript"});
const blobUrl = URL.createObjectURL(blob);

await import(blobUrl);

URL.revokeObjectURL(blobUrl);
await import(blobUrl);

But it should not be possible to import a revoked blob URL from a worker in which it has never been imported, even if it has been imported in a different thread. Deno will import it anyway, though:

// main.js
const worker = new Worker(
    new URL("./worker.js", import.meta.url),
    {type: "module"}
);

const blob = new Blob([`console.log("Hi from blob!");`], {type: "text/javascript"});
const blobUrl = URL.createObjectURL(blob);

await import(blobUrl);

URL.revokeObjectURL(blobUrl);
worker.postMessage(blobUrl);
// worker.js
self.addEventListener("message", async (evt) => {
    console.log("Importing from worker");
    await import(evt.data);  // should reject
});

This seems to be caused because Deno CLI has a cache of module sources, so that if the same module is imported from different workers, it doesn't result in multiple fetches. But since the cache is keyed by the URL, it doesn't take into consideration whether the blob has since been revoked.

This was found while investigating #12247, and a solution to this issue would have to consider that one as well.

@andreubotella
Copy link
Contributor Author

It seems like the blob resolution has to happen in CliModuleLoader::resolve(), since that method corresponds to the spec's "resolve a module specifier", which is where the URL gets parsed and the blob resolved. But that method returns a module specifier, so unless we want to modify the ModuleLoader trait in deno_core, a specifier → blob mapping must be stored somewhere, to be retrieved when prepare_load() and load() are called. Since CliModuleLoader instances seem to be specific to a worker, it makes sense to store the mapping there, rather than in ProcState.

ProcState's module map can then be changed to have its keys be either URLs or blob references (or maybe URLs plus optional blob references). This would solve this issue, since calling import() from a different worker will call the resolve() method of a different instance of CliModuleLoader, which will resolve the blob URL again rather than use the cached blob in the other instance. So if the URL has since been revoked, the module map key would only be a URL, not a blob. The blob would then have to be passed to GraphBuilder in order to fetch from it.

Implementing this would need some care to avoid leaking the blobs. In particular, the specifier → blob map in CliModuleLoader would need to keep a strong reference, since the import must succeed even if the object URL is revoked synchronously after calling import() (see #12247), but it should not keep it after CliModuleLoader::load() is called. Likewise, ProcState must be able to compare blob references with those in its module map for pointer equality, but it should not keep strong references.

I haven't dealt much with the module resolution code so far, and I probably won't have time to work any further on this until late next week, but I've written down what I have so far in case someone wants to take it.

@kitsonk
Copy link
Contributor

kitsonk commented Oct 1, 2021

I am in the process of doing a big refactor of the module loading in module_graph.rs and specifier_handler.rs migrating everything over to deno_graph. I would suggest that we try to get that done first before trying to resolve this issue.

moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Oct 4, 2021
…s across agents, a=testonly

Automatic update from web-platform-tests
Add tests for importing revoked blob URLs across agents (#30986)

Ref: denoland/deno#12248
--

wpt-commits: a9327c5dee0c2428e480cc78bdf6542a2af8fc63
wpt-pr: 30986
jamienicol pushed a commit to jamienicol/gecko that referenced this issue Oct 6, 2021
…s across agents, a=testonly

Automatic update from web-platform-tests
Add tests for importing revoked blob URLs across agents (#30986)

Ref: denoland/deno#12248
--

wpt-commits: a9327c5dee0c2428e480cc78bdf6542a2af8fc63
wpt-pr: 30986
@bartlomieju bartlomieju added bug Something isn't working correctly web related to Web APIs labels Oct 13, 2021
@exside
Copy link
Contributor

exside commented Oct 31, 2024

I'm not 100% sure this is related, but I'm currently dealing with an ever growing deps/blob folder in my project root (which I can easily delete) due to me creating a blob URL from a block of text/javascript I'm compiling in code like:

const components = URL.createObjectURL(new Blob([components.join('\n')], { type: 'application/javascript' }));

later on I call revokeObjectUrl:

URL.revokeObjectURL(components);

where I have expected that this would "kill" the blob, not sure what exactly it does, but neither the related file in deps/blob is removed nor is the variable in my example (components) null or undefined or whatever it should be, when logging components before and after revoking the blob URL it logs the exact same value.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working correctly web related to Web APIs
Projects
None yet
Development

No branches or pull requests

4 participants