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

Fix deadlock on process termination #1

Closed
wants to merge 1 commit into from
Closed

Conversation

Shulyaka
Copy link
Owner

Fixes rhasspy#29

From the asyncio.subprocess.wait specification:

Note: This method can deadlock when using stdout=PIPE or stderr=PIPE and the child process generates so much output that it blocks waiting for the OS pipe buffer to accept more data. Use the communicate() method when using pipes to avoid this condition.

This is exactly what we are doing.

  1. Start the process with stdout=PIPE
  2. Example for the mic program: read from the process until ASR is done
  3. Signal the process to stop and wait till it finishes

If between 2 and 3 the process outputs more data (as mic continues to record the sound), it creates a deadlock: we wait for the process to finish and the process waits for us to read the data he had already sent us.
This PR fixes that race condition as recommended in the documentation, by using communicate(), which will read all the data and then wait for the process to terminate.

@Shulyaka Shulyaka closed this Aug 21, 2023
@Shulyaka
Copy link
Owner Author

Wrong repo

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.

write() failed: Broken pipe
1 participant