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

FEATURE REQUEST: Callback or blocking method to wait for exit of spawned process #72

Closed
jakob-ledermann opened this issue Mar 13, 2024 · 3 comments · Fixed by #79
Closed
Milestone

Comments

@jakob-ledermann
Copy link

Currently there does not seem to be an easy way to wait for the spawned process to exit.

I do not see a better way to call the quit_cb callback, when the spawned process exits in https://github.com/jakob-ledermann/zellij/blob/445ee782e398aac34b4a4b7cedf386fa1d5c31f6/zellij-server/src/os_input_output.rs#L310

The unix implementation can be found here and uses .wait() to block until the child process has exited.

@andfoy
Copy link
Owner

andfoy commented Mar 19, 2024

Hi @jakob-ledermann, thanks for the suggestion. Right now the only way to test if a process is still "alive" is by trying to read from it in a blocking fashion, once the process finishes, the function will Err, which is similar to the functionality you are requesting for.

pub fn read(&self, length: u32, blocking: bool) -> Result<OsString, OsString> {

Ok(None) => Err(OsString::from("Standard out reached EOF")),

We could add a method wait or alike that could replicate the same functionality. However, from an implementation standpoint, I think it is clearer to handle the Err in the downstream function that performs the reading operation.

@jakob-ledermann
Copy link
Author

I agree that reacting to the error from read is currently the prefered option.
Unfortunately reading and detecting the termination are quite far apart in that code base so using the error to read to detect the termination is not trivial.

Replicating a wait function, that functions by reading the output of the process gets into conflicts if the user needs to read and process the output of the process.

Microsofts dotnet does provide an event Exited for a process, which can be used to register a callback to react to the termination of the process. That gave me the idea, that something simliar should be possible.

The Callback is registered with the Windows API in
https://github.com/microsoft/referencesource/blob/51cf7850defa8a17d815b4700b67116e3fa283c2/System/services/monitoring/system/diagnosticts/Process.cs#L1403-L1422

So it looks like a method .wait_for_exit() might actually be a call to WaitForSingleObject with the OS-Handle to the launched Process.

As far as I see there is currently no way to implement this outside the library as the native process handle can not be obtained.
Using the PID to obtain this handle is also not the best idea, as the PID is invalidated as soon as the process terminates. So you do run the risk of a data race for short running processes.

@andfoy
Copy link
Owner

andfoy commented Mar 20, 2024

Using WaitForSingleObject sounds more reasonable, thanks for the suggestion! I'll implement it for the next release

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 a pull request may close this issue.

2 participants