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

chore(workers): Test that closing a worker closes any child workers #12215

Merged
merged 2 commits into from
Sep 25, 2021

Conversation

andreubotella
Copy link
Contributor

Before #12156, closing a worker which had children would cause a panic (#11342 (comment)). After that PR, closing a worker will also close any child workers.

Andreu Botella added 2 commits September 25, 2021 01:14
Before denoland#12156, closing a worker which had children would cause a panic
(denoland#11342 (comment)).
After that PR, closing a worker will also close any child workers.
Copy link
Member

@piscisaureus piscisaureus left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for the test, @andreubotella.

@piscisaureus piscisaureus merged commit 1a6249c into denoland:main Sep 25, 2021
@andreubotella andreubotella deleted the closing-nested-worker-test branch September 25, 2021 02:27
@bartlomieju
Copy link
Member

After that PR, closing a worker will also close any child workers.

I need to go through the spec once more, but I believe this is actually where Deno diverges from the spec. IIRC in this case the children workers of the closed workers should be kept alive, but they should instead start targeting closed worker's parent.

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.

3 participants