Repository: Rapptz/discord.py
File: discord/player.py (Python)

Code before:
            return  # still running

        if getattr(self, '_stopped', False):
            return  # intentionally stopped

        if ret != 0 and self._current_error is None:
            # Only set error once, on first detection
            # read stderr if available
            stderr_text = None
            if self._stderr:
                try:
                    stderr_text = self._stderr.read(8192).decode(errors='ignore')
                except Exception:
                    stderr_text = '<failed to read stderr>'

            stderr_info = stderr_text if stderr_text else '<no stderr>'
            self._current_error = FFmpegError(f'FFmpeg exited with code {ret}. Stderr: {stderr_info}')

    def _kill_process(self) -> None:
        # check if FFmpeg process failed
        self._check_process_returncode()

        # this function gets called in __del__ so instance attributes might not even exist
        proc = getattr(self, '_process', MISSING)
        # Only proceed if proc is a subprocess.Popen instance
        if proc is MISSING or not hasattr(proc, 'kill'):
            return

        pid = getattr(proc, 'pid', 'unknown')
        _log.debug('Preparing to terminate ffmpeg process %s.', pid)

        try:
            proc.kill()
        except Exception:
            _log.exception('Ignoring error attempting to kill ffmpeg process %s', pid)

        if hasattr(proc, 'poll') and proc.poll() is None:
            _log.info('ffmpeg process %s has not terminated. Waiting to terminate...', pid)
            try:
                proc.communicate()
            except Exception:
                pass
            _log.info(
                'ffmpeg process %s should have terminated with a return code of %s.',
                pid,
                getattr(proc, 'returncode', 'unknown'),
            )
        else:
            _log.info(
                'ffmpeg process %s successfully terminated with return code of %s.',
                pid,

Review comment (suggestion):
This looks like an unnecessary change. `self._process` will be a subprocess.Popen instance if it's not MISSING.

Code after:
            return  # still running

        if self._stopped:
            return  # intentionally stopped

        if ret != 0 and self._current_error is None:
            # Only set error once, on first detection
            # read stderr if available
            stderr_text = None
            if self._stderr:
                try:
                    stderr_text = self._stderr.read(8192).decode(errors='ignore')
                except Exception:
                    stderr_text = '<failed to read stderr>'

            stderr_info = stderr_text if stderr_text else '<no stderr>'
            self._current_error = FFmpegProcessError(f'FFmpeg exited with code {ret}. Stderr: {stderr_info}')

    def _kill_process(self) -> None:
        # check if FFmpeg process failed
        self._check_process_returncode()

        # this function gets called in __del__ so instance attributes might not even exist
        proc = getattr(self, '_process', MISSING)
        # Only proceed if proc is a subprocess.Popen instance
        if proc is MISSING:
            return

        pid = getattr(proc, 'pid', 'unknown')
        _log.debug('Preparing to terminate ffmpeg process %s.', pid)

        try:
            proc.kill()
        except Exception:
            _log.exception('Ignoring error attempting to kill ffmpeg process %s', pid)

        try:
            still_running = proc.poll() is None
        except Exception:
            _log.exception('Error checking poll() on ffmpeg process %s', pid)
            still_running = False

        if still_running:
            _log.info('ffmpeg process %s has not terminated. Waiting to terminate...', pid)
            try:
                proc.communicate()
            except Exception:
                pass
            _log.info(
                'ffmpeg process %s should have terminated with a return code of %s.',
                pid,