ffdec: Stop the ffmpeg process before closing any buffers #78
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This fixes intermittent crashes that were introduced by commit ac7b1e0. In some situations the .close() method is called before the child process has completed. If we close proc.stderr and proc.stdout while a QueueReaderThread instance is still running, we get errors such as this: Exception in thread Thread-2: Traceback (most recent call last): File "/usr/lib64/python3.6/threading.py", line 916, in _bootstrap_inner self.run() File "/home/sam/src/audioread/audioread/ffdec.py", line 73, in run data = self.fh.read(self.blocksize) ValueError: I/O operation on closed file Sometimes the CPython interpreter segfaults inside the io.read() call before it can show this error. We now kill the child process and wait for it to terminate before closing the stdout and stderr pipes. This gives the queue reader threads time to receive the end-of-file notifications from the pipes while they are still open.
|
Wow! This is a great, simple fix to a subtle problem. Thank you for doing the investigation to figure this out! ✨ 🚀 |
sampsyo
added a commit
that referenced
this pull request
Dec 26, 2018
ffdec: Stop the ffmpeg process before closing any buffers
|
No problem. Thanks for all your work on Beets :-) |
This was referenced Dec 26, 2018
Merged
|
This PR fixed my testcase, but it hasn't actually fixed the issue when doing a |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This fixes intermittent crashes that were introduced by commit
ac7b1e0.
In some situations the .close() method is called before the child
process has completed. If we close proc.stderr and proc.stdout while
a QueueReaderThread instance is still running, we get errors such as
this:
Sometimes the CPython interpreter segfaults inside the io.read() call
before it can show this error.
We now kill the child process and wait for it to terminate before
closing the stdout and stderr pipes. This gives the queue reader threads
time to receive the end-of-file notifications from the pipes while they
are still open.