diff --git a/ocrmypdf/__main__.py b/ocrmypdf/__main__.py index 4ecbc381..38c32cb0 100755 --- a/ocrmypdf/__main__.py +++ b/ocrmypdf/__main__.py @@ -478,10 +478,58 @@ def traverse_ruffus_exception(e_args, options, log): return traverse_ruffus_exception(exc, options, log) +def check_closed_streams(options): + """Work around Python issue with multiprocessing forking on closed streams + + https://bugs.python.org/issue28326 + + Attempting to a fork/exec a new Python process when any of std{in,out,err} + are closed or not flushable for some reason may raise an exception. + Fix this by opening devnull if the handle seems to be closed. Do this + globally to avoid tracking places all places that fork. + + Seems to be specific to multiprocessing.Process not all Python process + forkers. + + The error actually occurs when the stream object is not flushable, + but replacing an open stream object that is not flushable with + /dev/null is a bad idea since it will create a silent failure. Replacing + a closed handle with /dev/null seems safe. + + """ + + if sys.stderr is None: + sys.stderr = open(os.devnull, 'w') + + if sys.stdin is None: + if options.input_file == '-': + print("Trying to read from stdin but stdin seems closed", + file=sys.stderr) + return False + sys.stdin = open(os.devnull, 'r') + + if sys.stdout is None: + if options.output_file == '-': + # Can't replace stdout if the user is piping + # If this case can even happen, it must be some kind of weird + # stream. + print(textwrap.dedent("""\ + Output was set to stdout '-' but the stream attached to + stdout does not support the flush() system call. This + will fail."""), file=sys.stderr) + return False + sys.stdout = open(os.devnull, 'w') + + return True + + def run_pipeline(): options = parser.parse_args() options.verbose_abbreviated_path = 1 + if not check_closed_streams(options): + return ExitCode.bad_args + _log, _log_mutex = proxy_logger.make_shared_logger_and_proxy( logging_factory, __name__, [None, options.verbose]) _log.debug('ocrmypdf ' + VERSION) @@ -522,9 +570,9 @@ def run_pipeline(): file.""")) return ExitCode.bad_args elif not is_file_writable(options.output_file): - _log.error(textwrap.dedent("""\ - Cutput file location is not writable.""")) - return ExitCode.file_access_error + _log.error(textwrap.dedent("""\ + Cutput file location is not writable.""")) + return ExitCode.file_access_error manager = JobContextManager() manager.register('JobContext', JobContext) diff --git a/tests/test_main.py b/tests/test_main.py index 7ed2a920..15d161f4 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -661,6 +661,23 @@ def test_stdout(spoof_tesseract_noop, ocrmypdf_exec, resources, outpdf): assert qpdf.check(output_file, log=None) +def test_closed_streams(spoof_tesseract_noop, ocrmypdf_exec, resources, outpdf): + input_file = str(resources / 'francais.pdf') + output_file = str(outpdf) + + def evil_closer(): + os.close(0) + os.close(1) + + p_args = ocrmypdf_exec + [input_file, output_file] + p = Popen( + p_args, close_fds=True, stdout=None, stderr=PIPE, stdin=None, + env=spoof_tesseract_noop, preexec_fn=evil_closer) + out, err = p.communicate() + print(err.decode()) + assert p.returncode == ExitCode.ok + + def test_masks(spoof_tesseract_noop, resources, outpdf): check_ocrmypdf(resources / 'masks.pdf', outpdf, env=spoof_tesseract_noop)