diff options
| -rw-r--r-- | youtube_dl/compat.py | 34 | ||||
| -rw-r--r-- | youtube_dl/downloader/external.py | 35 |
2 files changed, 51 insertions, 18 deletions
diff --git a/youtube_dl/compat.py b/youtube_dl/compat.py index 75dff58f2..53ff2a892 100644 --- a/youtube_dl/compat.py +++ b/youtube_dl/compat.py @@ -2438,9 +2438,9 @@ compat_html_parser_HTMLParser = compat_HTMLParser compat_html_parser_HTMLParseError = compat_HTMLParseError try: - from subprocess import DEVNULL - compat_subprocess_get_DEVNULL = lambda: DEVNULL -except ImportError: + _DEVNULL = subprocess.DEVNULL + compat_subprocess_get_DEVNULL = lambda: _DEVNULL +except AttributeError: compat_subprocess_get_DEVNULL = lambda: open(os.path.devnull, 'w') try: @@ -2958,6 +2958,33 @@ except ImportError: return exc_val is not None and isinstance(exc_val, self._exceptions or tuple()) +# subprocess.Popen context manager +# avoids leaking handles if .communicate() is not called +try: + _Popen = subprocess.Popen + # check for required context manager attributes + _Popen.__enter__ and _Popen.__exit__ + compat_subprocess_Popen = _Popen +except AttributeError: + # not a context manager - make one + from contextlib import contextmanager + + @contextmanager + def compat_subprocess_Popen(*args, **kwargs): + popen = None + try: + popen = _Popen(*args, **kwargs) + yield popen + finally: + if popen: + for f in (popen.stdin, popen.stdout, popen.stderr): + if f: + # repeated .close() is OK, but just in case + with compat_contextlib_suppress(EnvironmentError): + f.close() + popen.wait() + + # Fix https://github.com/ytdl-org/youtube-dl/issues/4223 # See http://bugs.python.org/issue9161 for what is broken def workaround_optparse_bug9161(): @@ -3314,6 +3341,7 @@ __all__ = [ 'compat_struct_pack', 'compat_struct_unpack', 'compat_subprocess_get_DEVNULL', + 'compat_subprocess_Popen', 'compat_tokenize_tokenize', 'compat_urllib_error', 'compat_urllib_parse', diff --git a/youtube_dl/downloader/external.py b/youtube_dl/downloader/external.py index bc228960e..f22fa6013 100644 --- a/youtube_dl/downloader/external.py +++ b/youtube_dl/downloader/external.py @@ -11,6 +11,7 @@ from .common import FileDownloader from ..compat import ( compat_setenv, compat_str, + compat_subprocess_Popen, ) from ..postprocessor.ffmpeg import FFmpegPostProcessor, EXT_TO_OUT_FORMATS from ..utils import ( @@ -483,21 +484,25 @@ class FFmpegFD(ExternalFD): self._debug_cmd(args) - proc = subprocess.Popen(args, stdin=subprocess.PIPE, env=env) - try: - retval = proc.wait() - except BaseException as e: - # subprocess.run would send the SIGKILL signal to ffmpeg and the - # mp4 file couldn't be played, but if we ask ffmpeg to quit it - # produces a file that is playable (this is mostly useful for live - # streams). Note that Windows is not affected and produces playable - # files (see https://github.com/ytdl-org/youtube-dl/issues/8300). - if isinstance(e, KeyboardInterrupt) and sys.platform != 'win32': - process_communicate_or_kill(proc, b'q') - else: - proc.kill() - proc.wait() - raise + # From [1], a PIPE opened in Popen() should be closed, unless + # .communicate() is called. Avoid leaking any PIPEs by using Popen + # as a context manager (newer Python 3.x and compat) + # Fixes "Resource Warning" in test/test_downloader_external.py + # [1] https://devpress.csdn.net/python/62fde12d7e66823466192e48.html + with compat_subprocess_Popen(args, stdin=subprocess.PIPE, env=env) as proc: + try: + retval = proc.wait() + except BaseException as e: + # subprocess.run would send the SIGKILL signal to ffmpeg and the + # mp4 file couldn't be played, but if we ask ffmpeg to quit it + # produces a file that is playable (this is mostly useful for live + # streams). Note that Windows is not affected and produces playable + # files (see https://github.com/ytdl-org/youtube-dl/issues/8300). + if isinstance(e, KeyboardInterrupt) and sys.platform != 'win32': + process_communicate_or_kill(proc, b'q') + else: + proc.kill() + raise return retval |