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

[Bug]: MessagePort to MessagePortMain cannot transfer transferable resources #34905

Open
3 tasks done
q8B opened this issue Jul 13, 2022 · 4 comments
Open
3 tasks done
Labels
18-x-y 19-x-y 20-x-y enhancement ✨ has-repro-gist Issue can be reproduced with code at https://gist.github.com/ platform/all

Comments

@q8B
Copy link

q8B commented Jul 13, 2022

Preflight Checklist

Electron Version

18, 19, 20

What operating system are you using?

Windows

Operating System Version

Windows 10 Enterprise 21H2 (19044.1766)

What arch are you using?

x64

Last Known Working Electron version

No response

Expected Behavior

As per documentation (https://www.electronjs.org/docs/latest/tutorial/message-ports) MessagePortMain should behave like the original MessagePort available in the renderer process, except for how the events are handled.

It's expected that a transferable object would be correctly passed to the main process when transferred from the renderer process without the need of serialization and deserialization of the object.

Actual Behavior

When transfering a transferable object, such as a buffer array, from the Renderer to Main process the entire message data is lost. Transferable object ownership is correctly lost in the renderer.

Testcase Gist URL

https://gist.github.com/a76896c0052ac3311fbe66436befefd0

Additional Information

The issue is easy to replicate (see the linked gist).

When the port is forwarded to another renderer process the transferable data is handled correctly, the issue is only from Renderer to Main

Looking at MessagePortMain's postMessage interface I can see that only MessagePortMain are accepted as transferables, but I can't find anywhere that says that it shouldn't be possible to transfer transferables fro Renderer's postMessage API to MessagePortMain message event

@q8B q8B added the bug 🪲 label Jul 13, 2022
@VerteDinde VerteDinde added platform/windows has-repro-gist Issue can be reproduced with code at https://gist.github.com/ 18-x-y 19-x-y 20-x-y labels Jul 13, 2022
@nornagon
Copy link
Member

nornagon commented Jul 13, 2022

Yeah, we currently only support transferring ports.

Code in Electron:

void WebContents::ReceivePostMessage(
const std::string& channel,
blink::TransferableMessage message,
content::RenderFrameHost* render_frame_host) {
v8::Isolate* isolate = JavascriptEnvironment::GetIsolate();
v8::HandleScope handle_scope(isolate);
auto wrapped_ports =
MessagePort::EntanglePorts(isolate, std::move(message.ports));
v8::Local<v8::Value> message_value =
electron::DeserializeV8Value(isolate, message);
EmitWithSender("-ipc-ports", render_frame_host,
electron::mojom::ElectronApiIPC::InvokeCallback(), false,
channel, message_value, std::move(wrapped_ports));
}

Code in Chromium showing other values that we don't read:
https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/public/common/messaging/transferable_message.h;l=33-35;drc=4b3d38a7f859236b34512076dfceb0202b4bf8e1

it'd probably be a reasonable addition to make array buffers work. here's the relevant code on the Blink side that does it: https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/messaging/blink_transferable_message.cc;l=61-71;drc=a6fe0210768868959ac7e8d0e04eaf771e83e524

That said, I'm not sure it's terribly useful? The data is copied either way, and there's no way to use shared memory between processes. I don't think this is more performant than not "transferring" the buffer.

[EDIT]: Here's a slightly simplified version of your gist that additionally exits with 0/1 depending on whether the test passes or fails. https://gist.github.com/865df64a9f3b4d9270b993156576a44e

@github-actions
Copy link
Contributor

This issue has been automatically marked as stale. If this issue is still affecting you, please leave any comment (for example, "bump"), and we'll keep it open. If you have any new additional information—in particular, if this is still reproducible in the latest version of Electron or in the beta—please include it with your comment!

@RealAlphabet
Copy link

Is there any progress on this?

@yume-chan
Copy link

The support for transfering MessagePorts is incomplete. If a message contains a MessagePort, the message will become null in main process.

Electron Fiddle gist: https://gist.github.com/yume-chan/92284827ce898045a82748d7c3f3ee4a

In Web I can do

const channel = new MessageChannel();
channel.port1.addEventListener("message", e=>{
    console.log("message",e.data,e.ports);
});
channel.port1.start();

const channel2 = new MessageChannel();
channel.port2.postMessage({ type: "without" }, [channel2.port1]);

const channel3 = new MessageChannel();
channel.port2.postMessage({ type: "with", port: channel3.port1 }, [channel3.port1]);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
18-x-y 19-x-y 20-x-y enhancement ✨ has-repro-gist Issue can be reproduced with code at https://gist.github.com/ platform/all
Projects
No open projects
Status: Does Not Block Stable
Development

No branches or pull requests

5 participants