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

Fix panic during shutdown when using channel #934

Closed
wants to merge 2 commits into from

Conversation

rf-figma
Copy link

This PR reverts the revert of #932, adding a second commit that should fix a related race condition that was causing a build failure.

rf- added 2 commits November 4, 2022 16:25
This fixes a bug that can be triggered when there's a JsBox holding an
instance of Channel at the moment the VM stops (e.g., when quitting an
Electron app). The Channel's `drop` implementation calls `self.send`,
which panics with a SendError because we're already in the middle of
tearing everything down.
This fixes a sporadic panic caused by a race condition between a Worker
loading our test-specific Rust extension and the worker being shut down.
This shouldn't panic in principle, but fixing it would require
introducing a leak (see 2bf1525 for the change we could have made and
decided not to).
setTimeout(cb, 100);
worker.once("message", async () => {
await worker.terminate();
cb();
Copy link
Member

@kjvalencik kjvalencik Nov 8, 2022

Choose a reason for hiding this comment

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

I believe this still needs to be delayed a bit for the race, but to be honest, I don't exactly remember the ordering of this test.

@kjvalencik
Copy link
Member

Merged via 000b6c5

@kjvalencik kjvalencik closed this Nov 9, 2022
@kjvalencik kjvalencik changed the title Fix two cases that can panic during shutdown Fix panic during shutdown when using channel Nov 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants