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

v8.deserialize on WebAssembly module fails with "Unable to deserialize cloned data" #18265

Closed
jayphelps opened this issue Jan 20, 2018 · 8 comments
Labels
v8 engine Issues and PRs related to the V8 dependency.

Comments

@jayphelps
Copy link

jayphelps commented Jan 20, 2018

  • Version: v9.4.0
  • Platform: 16.6.0 Darwin Kernel Version 16.6.0; root:xnu-3789.60.24~6/RELEASE_X86_64 x86_64
  • Subsystem: Serialization API (experimental)

Online REPL Reproduction

Using the experimental Serialization API calling v8.serialize(module) on a WebAssembly module works, however trying to then deserialize it v8.deserialize(buffer) fails with Error: Unable to deserialize cloned data..

From what I gather, node is using a newly exposed API from v8, so it's possible that exposed API is to blame. Although, v8 itself supports it within its internal APIs and the --wasm_disable_structured_cloning flag is exposed in node v9.4.0 with the expected value of false (don't disable aka is enabled)

node --v8-options

...

--wasm_disable_structured_cloning (disable wasm structured cloning)
        type: bool  default: false

# false means structured cloning aka serialization is NOT disabled AFAIK -jayphelps
const v8 = require('v8');

// only putting a simple wasm binary inline so it's easy to reproduce
const theModule = new WebAssembly.Module(new Uint8Array([
  0x00, 0x61, 0x73, 0x6d, 0x01, 0x00, 0x00, 0x00
]));

const buffer = v8.serialize(theModule);
v8.deserialize(buffer);
// "Error: Unable to deserialize cloned data."

A bit more real world example, for those curious:

const v8 = require('v8');
const fs = require('fs');

function writeDemo() {
  const module = new WebAssembly.Module(new Uint8Array([
    0x00, 0x61, 0x73, 0x6d, 0x01, 0x00, 0x00, 0x00
  ]));
  const buffer = v8.serialize(module);
  fs.writeFileSync('cached-wasm-module.buffer', buffer);
}

function readDemo() {
  const buffer = fs.readFileSync('cached-wasm-module.buffer');
  const module = new WebAssembly.Module(v8.deserialize(buffer));
  const instance = new WebAssembly.Instance(module);
}

writeDemo();
readDemo();

This is supported in browsers via postMessage or saving them into IndexedDB


No urgency on my part, just logging mostly curious if someone can point me in the right direction codewise and I'll take a peek. I went down the rabbit hole on my own and didn't immediately see anything that would have prohibited support.

Cc/ @kwonoj

@TimothyGu
Copy link
Member

Some printf debugging reveals that explicitly allowing WASM deserialization from source (over in-memory transfer) is necessary for this to work. I can't see why we can't allow it by default for v8.Deserializer.

/cc @addaleax

@bnoordhuis
Copy link
Member

Just flipping the flag isn't enough, we also need to implement ValueSerializer::GetWasmModuleTransferId().

We can probably do that the same way we implement ValueSerializer::GetSharedArrayBufferId(), by punting it to the user (through the serializer._getSharedArrayBufferId() callback.)

That said, I remember there were a bunch of issues with WASM serialization. I don't know if V8 6.2 is in a shape where it actually works.

@targos targos added the v8 engine Issues and PRs related to the V8 dependency. label Feb 24, 2018
@saschanaz
Copy link

Looks like this has been fixed now. https://runkit.com/saschanaz/5c84667c6e2ea0001222a59f

@jayphelps
Copy link
Author

Woot woot! Thanks for letting us know—and to whoever fixed it.

@kripken
Copy link

kripken commented May 2, 2020

It looks like this has regressed, the testcase here does not work on v14.1.0.

Same testcase, but with adding

console.log('serlialized length:', buffer.length);

between the serialization and deserialization, shows that with v14.1.0 the serialized size is just 2 bytes. I see the same for all modules, regardless of the size, using Emscripten's code caching option that is built on this.

For comparison on v12.9.1 everything works properly (and the serialized size is 38 bytes in this example).

Should I file a new issue?

@saschanaz
Copy link

True, it doesn't work anymore in v14.1.0 while it works on v13.14.0. https://runkit.com/saschanaz/5ead9385cf032a001e521bc9

@addaleax
Copy link
Member

addaleax commented May 2, 2020

@kripken @saschanaz This has happened due to large changes to how the V8 API deals with compiled WASM modules – the new interfaces here are quite complex, and I’m not sure whether the APIs can be mapped against each other easily (and, in particular, synchronously).

@kripken
Copy link

kripken commented May 3, 2020

@addaleax Oh, that's too bad... this was really useful.

An async API would be fine for the emscripten use cases, if that's possible!

kripken added a commit to emscripten-core/emscripten that referenced this issue May 4, 2020
    Note that this is broken on newer node, and mention where it works.
    see nodejs/node#18265

    Show a proper error in SINGLE_FILE mode, where this can't work.

    Add a missing initialization so we always work properly.

    Better error logging.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v8 engine Issues and PRs related to the V8 dependency.
Projects
None yet
Development

No branches or pull requests

7 participants