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.

Sign up for GitHub

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

Jump to bottom

feat: terminate spawned processes when the deno task process is terminated on Windows #35

Merged
merged 6 commits into from
Nov 30, 2024
Merged

feat: terminate spawned processes when the deno task process is terminated on Windows #35

merged 6 commits into from
Nov 30, 2024

Conversation

dsherret
Copy link
Member

I have a test for this in the CLI here: denoland/deno#14421 -- I wasn't sure how to test this here.

This uses the jobs api in Windows in order to accomplish this.

Closes #33

@dsherret dsherret requested a review from piscisaureus April 27, 2022 22:32
@dsherret dsherret mentioned this pull request Apr 27, 2022
Closed
piscisaureus
piscisaureus reviewed May 26, 2022
View reviewed changes
src/child_process_tracker.rs Outdated
let handle = CreateJobObjectW(ptr::null_mut(), ptr::null());
let mut info: JOBOBJECT_EXTENDED_LIMIT_INFORMATION = std::mem::zeroed();
info.BasicLimitInformation.LimitFlags =
JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE;
Copy link
Member

Choose a reason for hiding this comment

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

I would suggest to add JOB_OBJECT_LIMIT_BREAKAWAY_OK | JOB_OBJECT_LIMIT_SILENT_BREAKAWAY_OK.

This allows subprocesses to spawn other programs that run in the background.

Copy link
Member

Choose a reason for hiding this comment

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

  • JOB_OBJECT_LIMIT_DIE_ON_UNHANDLED_EXCEPTION

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks! Added all three.

@piscisaureus
Copy link
Member

I wasn't sure how to test this here.

If you need ideas...

  • spawn a process that never exits (e.g. cmd.exe /c pause)
  • add the process handle to the job object
  • drop the job objec - this should kill the process
  • wait for the process handle. Without the job object this should hang. With it, it should return.

piscisaureus
piscisaureus approved these changes May 26, 2022
View reviewed changes
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 (but see comments)

@mo mo mentioned this pull request Nov 12, 2024
Closed
bartlomieju
bartlomieju approved these changes Nov 30, 2024
View reviewed changes
Copy link
Member

@bartlomieju bartlomieju left a comment

Choose a reason for hiding this comment

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

LGTM

@dsherret dsherret merged commit f0df1f2 into denoland:main Nov 30, 2024
4 checks passed
@dsherret dsherret deleted the feat_terminate_sub_processes_when_parent_terminated branch November 30, 2024 00:10
@dsherret dsherret mentioned this pull request Nov 30, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bartlomieju bartlomieju bartlomieju approved these changes

@piscisaureus piscisaureus piscisaureus approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Deno processes spawned using deno task do not exit when the parent process exits on Windows
3 participants
@dsherret @piscisaureus @bartlomieju