search

beetbox / audioread

cross-library (GStreamer + Core Audio + MAD + FFmpeg) audio decoding for Python
MIT License
481 stars 108 forks source link

ffdec: Stop the ffmpeg process before closing any buffers #78

Closed ssssam closed 5 years ago

ssssam commented 5 years ago

This fixes intermittent crashes that were introduced by commit ac7b1e07ea47a1ebfc.

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.

sampsyo commented 5 years ago

Wow! This is a great, simple fix to a subtle problem. Thank you for doing the investigation to figure this out! :sparkles: :rocket:

ssssam commented 5 years ago

No problem. Thanks for all your work on Beets :-)

ssssam commented 5 years ago

This PR fixed my testcase, but it hasn't actually fixed the issue when doing a beet import :-(