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

Use BroadcastChannel to support sub workers #25

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

rchiodo
Copy link

@rchiodo rchiodo commented Oct 10, 2022

Fixes #23

Switch to using a BroadcastChannel to allow sub workers to participate in sync messages too.

}

dispose() {
this.channel.removeEventListener('message', this._handleMessageEvent.bind(this));
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not happy with requiring a dispose, but since Broadcast channels are global, they need to be cleaned up (don't go away on Worker shutdown)

An alternative would be to force a new channel name on each Client/Server pair, but that would be hard to reconcile because the Client is created in the worker and doesn't have access to the memory of the Server.

Additionally, the BroadcastChannel only works on the same origin. Would the same origin be used for the extension host as the workers in vscode.dev?

@rchiodo
Copy link
Author

rchiodo commented Oct 11, 2022

Okay maybe this isn't going to work in the browser. Seems that SharedArrayBuffers can't be passed over BroadcastChannels. They only work over MessagePorts.

https://stackoverflow.com/questions/63627423/is-it-possible-to-broadcast-a-sharedarraybuffer-to-web-workers-via-the-broadcast

Perhaps a BroadcastChannel can be used to send a message port to the main thread somehow. Then the MessagePort can be used to send messages the way the code used to work

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 this pull request may close these issues.

Sync-Api doesn't work with multiple workers
1 participant