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

Make subprocess async read non-blocking #76

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

legerch
Copy link

@legerch legerch commented Sep 26, 2023

When using option "subprocess_option_enable_async", subprocess was still blocking when calling "subprocess_read_stdout()" and no data were available.

Now, subprocess_read_stdout() will return even when no available data, returned value will be equal to 0.
(Note that I only made modification for non-Windows filesystems)

When using option "subprocess_option_enable_async", subprocess was still blocking when calling "subprocess_read_stdout()" and no data were available.

Now, subprocess_read_stdout() will return even when no available data, returned value will be equal to 0.
@sheredom
Copy link
Owner

This needs a test case to show why the behaviour was needed before I could land it.

@legerch
Copy link
Author

legerch commented Sep 27, 2023

This needs a test case to show why the behaviour was needed before I could land it.

In my case, this was needed because my main event loop was blocked by this call (subprocess_read_stdout()) since my running subprocess is quite long and doesn't send much to stdout). So I was unable to perform other tasks while waiting for stdout data (like feeding the watchdog for example).

But I'm not sure how to update tests... since they are waiting than read data reach 0 (but this is a blocking event)

@sheredom
Copy link
Owner

I think if we used one of the process helpers that waits on stdin, we could have a test that launches a process async, reads stdout (asserts that it read nothing), then writes to stdin to unblock the process. I think that'd be enough?

I see some other tests fail, most likely because they rely on the async read returning something (even if its returned early). Those tests might have to change to loop until the data is received?

@legerch
Copy link
Author

legerch commented Sep 27, 2023

I think if we used one of the process helpers that waits on stdin, we could have a test that launches a process async, reads stdout (asserts that it read nothing), then writes to stdin to unblock the process. I think that'd be enough?

This is a good idea, I pushed a commit with this behaviour. Is it what you had in mind ? :)

I see some other tests fail, most likely because they rely on the async read returning something (even if its returned early). Those tests might have to change to loop until the data is received?

I have fixed those tests by replacing loop condition while(bytes_read != 0) by while(subprocess_still_alive())

EDIT: I don't understand, when I run test on local, everything is ok but here on CI pipeline I have error (do you know if CI use particular flags, toolchains ?)

@legerch legerch force-pushed the make-read-non-blocking-async-option branch from 62fd4b1 to b65dbc0 Compare September 27, 2023 09:37
@rasky
Copy link
Contributor

rasky commented Jan 12, 2024

I found this PR while trying to understand what subprocess_option_enable_async means.

In my code, I use subprocess to spawn an interactive process with a "shell". I can send commands writing to its standard input, terminated by "\n", and then must read the output written to its standard output. I'm currently not using subprocess_option_enable_async and it appears to work on all operating systems I tested (Windows, Linux, Mac), but the documentation as written seems to imply that I should use subprocess_option_enable_asyncif I try to interact with a process? If so, I most surely needs a blocking behavior.

Can anybody clarify to me the intended semantic of subprocess_option_enable_async?

@legerch
Copy link
Author

legerch commented Jan 12, 2024

Can anybody clarify to me the intended semantic of subprocess_option_enable_async?

Hello,
Flag subprocess_option_enable_async is useful when you need a non-blocking scenario. For example, subprocess simple scenario would be:

  1. Create a subprocess with subprocess_create() method (with options set to 0) : note that your subprocess (executed script) is started with this method
  2. Wait for subprocess termination with subprocess_join() : your calling code is "blocked" until your subprocess is finished
  3. Subprocess is finished, you can read its output with subprocess_stdout()or subprocess_stderr()

Now, assume our script write to stdout for each step completed and we want to display a progress bar from our calling code... We can't use subprocess_join() because this will wait subprocess termination, we want to read stdout as soon as something is available in order to display our progress bar...

This is when option subprocess_option_enable_async can be be useful:

  1. Create a subprocess with subprocess_create() method (with options set to subprocess_option_enable_async)
  2. While subprocess is alive (subprocess_alive()) :
    1. Check for pending datas with subprocess_read_stdout() or subprocess_read_stderr()
    2. If buffer read contains datas, parse it and update our progress bar

Main goal of this PR is because subprocess_read_stdout() will block until at least size byte has been read (so 0 will be returned only when subprocess is finished). This PR change this behaviour by allowing to return 0 (meaning: no data currently available), which allow to perform other task

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.

None yet

3 participants