-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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 in worker when closing at top level #8510
Conversation
let prefix = "file://"; | ||
if (Deno.build.os == "windows") { | ||
prefix += "/"; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aside: I wish had something like Deno.makeTempFile().toUrl()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// evaluation was complete. This can happen in Web Worker when `self.close()` | ||
// is called at top level. | ||
let result = maybe_result.unwrap_or(Ok(())); | ||
return Poll::Ready(result); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be really great to have a standalone test in core for this...
bff8f67
to
08a5e95
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Discussed offline - too difficult to get test in core.
LGTM
Fixes panic occurring in worker when
self.close()
is calledat the top level, ie. worker shuts down while
module evaluation promise hasn't yet resolved.
Closes #7620
Closes #5334
Supersedes #8395