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

Shared exe-unit process module #3168

Merged
merged 6 commits into from
Apr 15, 2024

Conversation

pwalski
Copy link
Contributor

@pwalski pwalski commented Apr 11, 2024

Resolves: #3167

@pwalski pwalski linked an issue Apr 11, 2024 that may be closed by this pull request
@pwalski pwalski force-pushed the pwalski/ya_process_crate branch 4 times, most recently from cd2223f to a059e97 Compare April 13, 2024 11:48
@pwalski pwalski marked this pull request as ready for review April 13, 2024 12:30
@pwalski pwalski requested a review from a team as a code owner April 13, 2024 12:30
@pwalski pwalski changed the title Shared process module Shared counters process module Apr 15, 2024
@pwalski pwalski changed the title Shared counters process module Shared exe-unit process module Apr 15, 2024
}

impl ProcessHandle {
pub fn new(command: &mut Command) -> Result<ProcessHandle> {
let process = Arc::new(SharedChild::spawn(command)?);
#[cfg(windows)]
let job_object = JobObject::try_new_current()?;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I left new_process_group unimplemented for Windows. process_group is roughly equivalent of Windows JobObject, but new_process_group is implemented for Command and I needed to store job_object somewhere, I could not extend Command, so instead I just crate job_object while creating ProcessHandle.

Comment on lines +104 to +106
#[allow(unused)]
#[cfg(windows)]
job_object: JobObject,
Copy link
Contributor

Choose a reason for hiding this comment

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

Having conditionally existing field is hard to maintain. I'm wondering if we could have empty job object implementation for unix?
In general all these conditional compilation in process handling is hard to maintain, because we need to compile with different features to be sure it will compile... But it was already done like this before, so we can leave it as is if it would require to much work.

@pwalski pwalski merged commit c16a6ba into staszek/gamerhash-combined Apr 15, 2024
11 checks passed
@pwalski pwalski deleted the pwalski/ya_process_crate branch April 15, 2024 11:47
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.

Create a JobObject when spawning a runtime on Windows
2 participants