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

MessagePort.postMessage with custom iterator as transfer list causes Assertion `(index) < (length())' failed. #49940

Closed
KhafraDev opened this issue Sep 28, 2023 · 3 comments · Fixed by #50398

Comments

@KhafraDev
Copy link
Member

KhafraDev commented Sep 28, 2023

Version

v21.0.0-pre (I assume every version w/ structuredClone)

Platform

Linux DESKTOP-L4O1H93 5.10.102.1-microsoft-standard-WSL2 #1 SMP Wed Mar 2 00:30:59 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux

Subsystem

No response

What steps will reproduce the bug?

structuredClone({}, {
  transfer: {
    *[Symbol.iterator]() {}
  }
})

or

new MessageChannel().port1.postMessage({}, {
  transfer: {
    *[Symbol.iterator]() {}
  }
})

How often does it reproduce? Is there a required condition?

No response

What is the expected behavior? Why is that the expected behavior?

this should work and return an empty object

What do you see instead?

./node[4717]: ../../src/util.h:392:T& node::MaybeStackBuffer<T, kStackStorageSize>::operator[](size_t) [with T = v8::Local<v8::Value>; long unsigned int kStackStorageSize = 8; size_t = long unsigned int]: Assertion `(index) < (length())' failed.
 1: 0x561c3fcd9e94 node::Abort() [./node]
 2: 0x561c3fcd9f28  [./node]
 3: 0x561c3fd3310c  [./node]
 4: 0x561c3fd35a4e node::worker::MessagePort::PostMessage(v8::FunctionCallbackInfo<v8::Value> const&) [./node]
 5: 0x561c3ffb4c00 v8::internal::FunctionCallbackArguments::Call(v8::internal::CallHandlerInfo) [./node]
 6: 0x561c3ffb54f9  [./node]
 7: 0x561c3ffb59e5 v8::internal::Builtin_HandleApiCall(int, unsigned long*, v8::internal::Isolate*) [./node]
 8: 0x561c40a14df6  [./node]
Aborted

Additional information

The webidl states that a sequence is basically anything with a Symbol.iterator, not exclusive to arrays. https://webidl.spec.whatwg.org/#es-sequence

@KhafraDev KhafraDev changed the title structuredClone with custom iterator as transfer list causes Assertion `(index) < (length())' failed. MessagePort.postMessage with custom iterator as transfer list causes Assertion `(index) < (length())' failed. Sep 28, 2023
@targos
Copy link
Member

targos commented Oct 2, 2023

Would it be spec-compliant to just convert the value to an array with Array.from before passing it to the C++ side? It would also take care of potential errors thrown during iteration.

@KhafraDev
Copy link
Member Author

not entirely, but it's better than nothing. For example, something that isn't null and doesn't have a Symbol.iterator won't throw with Array.from, but spec-wise is expected to (Array.from(3) // []).

@KhafraDev
Copy link
Member Author

KhafraDev commented Oct 2, 2023

someone could use

function createSequenceConverter(converter) {
which would be spec compliant, but afaik MessagePort is entirely native so I'm not sure how that would work.

or maybe using

static Maybe<bool> ReadIterable(Environment* env,
would work also, it looks compliant, and probably the reason that Workers don't throw when transferring the same object.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants