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

cannot post an Error, Map or Set between worker threads #3317

Closed
Fenzland opened this issue Nov 11, 2019 · 10 comments
Closed

cannot post an Error, Map or Set between worker threads #3317

Fenzland opened this issue Nov 11, 2019 · 10 comments

Comments

@Fenzland
Copy link
Contributor

Fenzland commented Nov 11, 2019

// index.js

const worker= new Worker( './worker.js', { name:'worker', type:'module', }, );

worker.onmessage= ( { data, }, )=> {
	console.assert( data.error instanceof Error, );
	console.assert( data.error.hasOwnProperty( 'stack', ), );
	console.assert( data.map instanceof Map, );
	console.assert( data.map.has( 'foo', ), );
	console.assert( data.map.get( 'foo', ) === 'bar', );
	console.assert( data.set instanceof Set, );
	console.assert( data.set.has( 'foo', ), );
};
worker.postMessage( {
	error: new Error(),
	map: new Map( [ [ 'foo', 'bar', ], ], ),
	set: new Set( [ 'foo', ], ),
}, );
// worker.js

globalThis.onmessage= ( { data, }, )=> {
	console.assert( data.error instanceof Error, );
	console.assert( data.error.hasOwnProperty( 'stack', ), );
	console.assert( data.map instanceof Map, );
	console.assert( data.map.has( 'foo', ), );
	console.assert( data.map.get( 'foo', ) === 'bar', );
	console.assert( data.set instanceof Set, );
	console.assert( data.set.has( 'foo', ), );
	
	globalThis.postMessage( {
		error: new Error(),
		map: new Map( [ [ 'foo', 'bar', ], ], ),
		set: new Set( [ 'foo', ], ),
	}, );
	
	globalThis.close();
};

In Chrome, all the assert successful. But in deno (until 0.23.0), they fail. After post between threads, errors, maps and sets become empty objects.

@kitsonk
Copy link
Contributor

kitsonk commented Nov 11, 2019

Hmmmmm... it seems that the structure clone on postMessage shouldn't be handling Error or Function objects. It feels like the browser must be filtering them out and cloning them in some other way: https://developer.mozilla.org/en-US/docs/Web/API/Web_Workers_API/Structured_clone_algorithm

I will admit I don't know how we implemented cloning in Deno for workers...

@kevinkassimo
Copy link
Contributor

@kitsonk we currently stringify data as JSON into some buffer that is passed directly to the Rust side and pushed as a message through a channel, where the receiving end polls and parses the JSON as data.

This is indeed not fully complying with structured clone requirements (e.g. transferables). But for Errors I would expect us to simply throw an error when attempted.

@Fenzland
Copy link
Contributor Author

JSON.stringify not only lose errors, also miss maps and sets. They are both pass correctly in browsers too.

@Fenzland Fenzland changed the title cannot post a Error between worker threads cannot post an Error, Map or Set between worker threads Nov 15, 2019
@kevinkassimo
Copy link
Contributor

@Fenzland I agree we will need to fix this. /cc @bartlomieju for worker stuff

@kitsonk
Copy link
Contributor

kitsonk commented Nov 15, 2019

There is the value serialiser in V8 that does the structured cloning: https://v8docs.nodesource.com/node-7.10/df/d48/classv8_1_1_value_serializer.html

I am sure there must be APIs too for transferring as well, I just haven't found them yet.

@bartlomieju
Copy link
Member

@kitsonk
Copy link
Contributor

kitsonk commented Nov 15, 2019

Well that and: https://denolib.github.io/v8-docs/classv8_1_1ValueSerializer.html so post should serialize the message and onmessage should be called with the deserialized message I think. I am not so hot at the V8 APIs though.

@bartlomieju
Copy link
Member

Makes sense. This issue (or more general workers) are in my backlog, but I don't know when I'll get to them. If someone wants to take a stab at it I'll be happy to provide support

@kevinkassimo
Copy link
Contributor

@bartlomieju Oh I almost forgot I still have that v8 docs up... it has not been updated for a while. I mostly just refer to https://cs.chromium.org/chromium/src/v8/include/v8.h , which is well updated and, since it is using Google's CodeSearch, easy to look up references and usages than on Github.

@kitsonk
Copy link
Contributor

kitsonk commented Dec 30, 2019

@bartlomieju we should close in favour of #3557 as that solution would solve all of this properly.

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

No branches or pull requests

4 participants