From 4d46360fb672684ff8b561c094e17759b67a197f Mon Sep 17 00:00:00 2001 From: Unrud Date: Mon, 14 Sep 2020 23:20:02 +0200 Subject: [PATCH] Kill child processes When the youtube-dl processes is killed, child processes like `ffmpeg` keep running in the background. The solution is similiar to [the implementation of subprocess.call](https://github.com/python/cpython/blob/75c1ca7b6c546ef674eb40bfe47f41e6045dc99b/Lib/subprocess.py#L250). The problem doesn't arise when youtube-dl runs in a terminal and Ctrl+C is pressed, because the terminal sends SIGINT to all processes of the process group. It becomes an issue when youtube-dl is started outside of a terminal or integrated into another program. --- youtube_dl/YoutubeDL.py | 3 ++- youtube_dl/compat.py | 3 ++- youtube_dl/downloader/external.py | 14 +++++++++----- youtube_dl/downloader/rtmp.py | 10 ++++++---- youtube_dl/extractor/openload.py | 3 ++- youtube_dl/postprocessor/embedthumbnail.py | 5 +++-- youtube_dl/postprocessor/ffmpeg.py | 5 +++-- youtube_dl/utils.py | 18 ++++++++++++++---- 8 files changed, 41 insertions(+), 20 deletions(-) diff --git a/youtube_dl/YoutubeDL.py b/youtube_dl/YoutubeDL.py index 19370f62b..c5d1dce71 100755 --- a/youtube_dl/YoutubeDL.py +++ b/youtube_dl/YoutubeDL.py @@ -93,6 +93,7 @@ from .utils import ( YoutubeDLCookieProcessor, YoutubeDLHandler, YoutubeDLRedirectHandler, + process_communicate_or_kill, ) from .cache import Cache from .extractor import get_info_extractor, gen_extractor_classes, _LAZY_LOADER @@ -2264,7 +2265,7 @@ class YoutubeDL(object): ['git', 'rev-parse', '--short', 'HEAD'], stdout=subprocess.PIPE, stderr=subprocess.PIPE, cwd=os.path.dirname(os.path.abspath(__file__))) - out, err = sp.communicate() + out, err = process_communicate_or_kill(sp) out = out.decode().strip() if re.match('[0-9a-f]+', out): self._write_string('[debug] Git HEAD: ' + out + '\n') diff --git a/youtube_dl/compat.py b/youtube_dl/compat.py index 0ee9bc760..bc65cda66 100644 --- a/youtube_dl/compat.py +++ b/youtube_dl/compat.py @@ -2874,6 +2874,7 @@ def workaround_optparse_bug9161(): if hasattr(shutil, 'get_terminal_size'): # Python >= 3.3 compat_get_terminal_size = shutil.get_terminal_size else: + from .utils import process_communicate_or_kill _terminal_size = collections.namedtuple('terminal_size', ['columns', 'lines']) def compat_get_terminal_size(fallback=(80, 24)): @@ -2893,7 +2894,7 @@ else: sp = subprocess.Popen( ['stty', 'size'], stdout=subprocess.PIPE, stderr=subprocess.PIPE) - out, err = sp.communicate() + out, err = process_communicate_or_kill(sp) _lines, _columns = map(int, out.split()) except Exception: _columns, _lines = _terminal_size(*fallback) diff --git a/youtube_dl/downloader/external.py b/youtube_dl/downloader/external.py index c31f8910a..b42e6a8f1 100644 --- a/youtube_dl/downloader/external.py +++ b/youtube_dl/downloader/external.py @@ -22,6 +22,7 @@ from ..utils import ( handle_youtubedl_headers, check_executable, is_outdated_version, + process_communicate_or_kill, ) @@ -104,7 +105,7 @@ class ExternalFD(FileDownloader): p = subprocess.Popen( cmd, stderr=subprocess.PIPE) - _, stderr = p.communicate() + _, stderr = process_communicate_or_kill(p) if p.returncode != 0: self.to_stderr(stderr.decode('utf-8', 'replace')) return p.returncode @@ -141,7 +142,7 @@ class CurlFD(ExternalFD): # curl writes the progress to stderr so don't capture it. p = subprocess.Popen(cmd) - p.communicate() + process_communicate_or_kill(p) return p.returncode @@ -336,14 +337,17 @@ class FFmpegFD(ExternalFD): proc = subprocess.Popen(args, stdin=subprocess.PIPE, env=env) try: retval = proc.wait() - except KeyboardInterrupt: + except BaseException as e: # subprocces.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 sys.platform != 'win32': - proc.communicate(b'q') + if isinstance(e, KeyboardInterrupt) and sys.platform != 'win32': + process_communicate_or_kill(proc, b'q') + else: + proc.kill() + proc.wait() raise return retval diff --git a/youtube_dl/downloader/rtmp.py b/youtube_dl/downloader/rtmp.py index fbb7f51b0..8a25dbc8d 100644 --- a/youtube_dl/downloader/rtmp.py +++ b/youtube_dl/downloader/rtmp.py @@ -89,11 +89,13 @@ class RtmpFD(FileDownloader): self.to_screen('') cursor_in_new_line = True self.to_screen('[rtmpdump] ' + line) - finally: + if not cursor_in_new_line: + self.to_screen('') + return proc.wait() + except BaseException: # Including KeyboardInterrupt + proc.kill() proc.wait() - if not cursor_in_new_line: - self.to_screen('') - return proc.returncode + raise url = info_dict['url'] player_url = info_dict.get('player_url') diff --git a/youtube_dl/extractor/openload.py b/youtube_dl/extractor/openload.py index 0c20d0177..dfdd0e526 100644 --- a/youtube_dl/extractor/openload.py +++ b/youtube_dl/extractor/openload.py @@ -17,6 +17,7 @@ from ..utils import ( get_exe_version, is_outdated_version, std_headers, + process_communicate_or_kill, ) @@ -226,7 +227,7 @@ class PhantomJSwrapper(object): self.exe, '--ssl-protocol=any', self._TMP_FILES['script'].name ], stdout=subprocess.PIPE, stderr=subprocess.PIPE) - out, err = p.communicate() + out, err = process_communicate_or_kill(p) if p.returncode != 0: raise ExtractorError( 'Executing JS failed\n:' + encodeArgument(err)) diff --git a/youtube_dl/postprocessor/embedthumbnail.py b/youtube_dl/postprocessor/embedthumbnail.py index 5a3359588..2bd4d5d05 100644 --- a/youtube_dl/postprocessor/embedthumbnail.py +++ b/youtube_dl/postprocessor/embedthumbnail.py @@ -14,7 +14,8 @@ from ..utils import ( PostProcessingError, prepend_extension, replace_extension, - shell_quote + shell_quote, + process_communicate_or_kill, ) @@ -105,7 +106,7 @@ class EmbedThumbnailPP(FFmpegPostProcessor): self._downloader.to_screen('[debug] AtomicParsley command line: %s' % shell_quote(cmd)) p = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE) - stdout, stderr = p.communicate() + stdout, stderr = process_communicate_or_kill(p) if p.returncode != 0: msg = stderr.decode('utf-8', 'replace').strip() diff --git a/youtube_dl/postprocessor/ffmpeg.py b/youtube_dl/postprocessor/ffmpeg.py index 5f7298345..04fd81e5e 100644 --- a/youtube_dl/postprocessor/ffmpeg.py +++ b/youtube_dl/postprocessor/ffmpeg.py @@ -21,6 +21,7 @@ from ..utils import ( dfxp2srt, ISO639Utils, replace_extension, + process_communicate_or_kill, ) @@ -180,7 +181,7 @@ class FFmpegPostProcessor(PostProcessor): handle = subprocess.Popen( cmd, stderr=subprocess.PIPE, stdout=subprocess.PIPE, stdin=subprocess.PIPE) - stdout_data, stderr_data = handle.communicate() + stdout_data, stderr_data = process_communicate_or_kill(handle) expected_ret = 0 if self.probe_available else 1 if handle.wait() != expected_ret: return None @@ -228,7 +229,7 @@ class FFmpegPostProcessor(PostProcessor): if self._downloader.params.get('verbose', False): self._downloader.to_screen('[debug] ffmpeg command line: %s' % shell_quote(cmd)) p = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, stdin=subprocess.PIPE) - stdout, stderr = p.communicate() + stdout, stderr = process_communicate_or_kill(p) if p.returncode != 0: stderr = stderr.decode('utf-8', 'replace') msg = stderr.strip().split('\n')[-1] diff --git a/youtube_dl/utils.py b/youtube_dl/utils.py index 01d9c0362..205b1e5f7 100644 --- a/youtube_dl/utils.py +++ b/youtube_dl/utils.py @@ -2211,6 +2211,15 @@ def unescapeHTML(s): r'&([^&;]+;)', lambda m: _htmlentity_transform(m.group(1)), s) +def process_communicate_or_kill(p, *args, **kwargs): + try: + return p.communicate(*args, **kwargs) + except BaseException: # Including KeyboardInterrupt + p.kill() + p.wait() + raise + + def get_subprocess_encoding(): if sys.platform == 'win32' and sys.getwindowsversion()[0] >= 5: # For subprocess calls, encode with locale encoding @@ -3720,7 +3729,8 @@ def check_executable(exe, args=[]): """ Checks if the given binary is installed somewhere in PATH, and returns its name. args can be a list of arguments for a short output (like -version) """ try: - subprocess.Popen([exe] + args, stdout=subprocess.PIPE, stderr=subprocess.PIPE).communicate() + process_communicate_or_kill(subprocess.Popen( + [exe] + args, stdout=subprocess.PIPE, stderr=subprocess.PIPE)) except OSError: return False return exe @@ -3734,10 +3744,10 @@ def get_exe_version(exe, args=['--version'], # STDIN should be redirected too. On UNIX-like systems, ffmpeg triggers # SIGTTOU if youtube-dl is run in the background. # See https://github.com/ytdl-org/youtube-dl/issues/955#issuecomment-209789656 - out, _ = subprocess.Popen( + out, _ = process_communicate_or_kill(subprocess.Popen( [encodeArgument(exe)] + args, stdin=subprocess.PIPE, - stdout=subprocess.PIPE, stderr=subprocess.STDOUT).communicate() + stdout=subprocess.PIPE, stderr=subprocess.STDOUT)) except OSError: return False if isinstance(out, bytes): # Python 2.x @@ -5675,7 +5685,7 @@ def write_xattr(path, key, value): cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, stdin=subprocess.PIPE) except EnvironmentError as e: raise XAttrMetadataError(e.errno, e.strerror) - stdout, stderr = p.communicate() + stdout, stderr = process_communicate_or_kill(p) stderr = stderr.decode('utf-8', 'replace') if p.returncode != 0: raise XAttrMetadataError(p.returncode, stderr)