aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--youtube_dl/compat.py34
-rw-r--r--youtube_dl/downloader/external.py35
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