From 612e0c1aa08d6500cff0e7c3f6d44d573329d932 Mon Sep 17 00:00:00 2001 From: Tobias Diez Date: Fri, 4 Dec 2020 13:29:30 +0100 Subject: [PATCH 1/5] Add traceback information to exceptions during docbuild --- src/sage_setup/docbuild/utils.py | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/src/sage_setup/docbuild/utils.py b/src/sage_setup/docbuild/utils.py index 056392ac39e..376fd8a6626 100644 --- a/src/sage_setup/docbuild/utils.py +++ b/src/sage_setup/docbuild/utils.py @@ -2,14 +2,17 @@ import errno import os +import sys +from types import TracebackType class WorkerDiedException(RuntimeError): """Raised if a worker process dies unexpected.""" - def __init__(self, message, original_exception=None): + def __init__(self, message, original_exception: BaseException = None, original_traceback: TracebackType = None): super(WorkerDiedException, self).__init__(message) self.original_exception = original_exception + self.original_traceback = original_traceback def build_many(target, args, processes=None): @@ -113,7 +116,7 @@ def build_many(target, args, processes=None): WorkerDiedException: worker for 4 died with non-zero exit code -9 """ import multiprocessing - from multiprocessing import Process, Queue, cpu_count, set_start_method + from multiprocessing import set_start_method # With OS X, Python 3.8 defaults to use 'spawn' instead of 'fork' # in multiprocessing, and Sage docbuilding doesn't work with # 'spawn'. See trac #27754. @@ -133,8 +136,8 @@ def build_many(target, args, processes=None): def run_worker(target, queue, idx, task): try: result = target(task) - except BaseException as exc: - queue.put((None, exc)) + except BaseException: + queue.put((None, sys.exc_info())) else: queue.put((idx, result)) @@ -154,13 +157,12 @@ def bring_out_yer_dead(w, task, exitcode): w._popen.returncode = exitcode if exitcode != 0: - raise WorkerDiedException( - "worker for {} died with non-zero exit code " - "{}".format(task[1], w.exitcode)) + raise WorkerDiedException(f"worker for {task[1]} died with non-zero exit code {w.exitcode}") # Get result from the queue; depending on ordering this may not be # *the* result for this worker, but for each completed worker there # should be *a* result so let's get it + result = None try: result = result_queue.get_nowait() except Empty: @@ -168,9 +170,13 @@ def bring_out_yer_dead(w, task, exitcode): # don't worry we'll collect any remaining results at the end. pass + if result is None: + return None + if result[0] is None: # Indicates that an exception occurred in the target function - raise WorkerDiedException('', original_exception=result[1]) + _, exception, tracback = result[1] + raise WorkerDiedException('', original_exception=exception, original_traceback=tracback) else: results.append(result) @@ -286,7 +292,7 @@ def reap_workers(waited_pid=None, waited_exitcode=None): # worker died unexpectedly, or the original exception if it's # wrapping one if worker_exc.original_exception: - raise worker_exc.original_exception + raise worker_exc.original_exception.with_traceback(worker_exc.original_traceback) else: raise worker_exc From f10a30c28aff52988557a461f80af6234fd24535 Mon Sep 17 00:00:00 2001 From: Tobias Diez Date: Sat, 9 Jan 2021 16:18:10 +0100 Subject: [PATCH 2/5] Don't try to prickle the complete traceback --- src/sage_setup/docbuild/utils.py | 63 ++++++++++++++++++++++++++------ 1 file changed, 52 insertions(+), 11 deletions(-) diff --git a/src/sage_setup/docbuild/utils.py b/src/sage_setup/docbuild/utils.py index 376fd8a6626..4b9bf4fda6e 100644 --- a/src/sage_setup/docbuild/utils.py +++ b/src/sage_setup/docbuild/utils.py @@ -2,17 +2,54 @@ import errno import os -import sys -from types import TracebackType +import traceback +from typing import Optional +class RemoteException(Exception): + """ + Raised if an exception occurred in one of the child processes. + """ + tb: str + def __init__(self, tb: str): + self.tb = tb + + def __str__(self): + return self.tb + +class RemoteExceptionWrapper: + """ + Used by child processes to capture exceptions thrown during execution and + report them to the main process, including the correct traceback. + """ + exc: BaseException + tb: str + + def __init__(self, exc: BaseException): + # We cannot pickle the traceback, thus convert it to a string. + # Later on unpickling, we set the original tracback as the cause of the exception + # This approach is taken from https://bugs.python.org/issue13831 + tb = traceback.format_exception(type(exc), exc, exc.__traceback__) + tb = ''.join(tb) + self.exc = exc + self.tb = f'\n"""\n{tb}"""' + + def __reduce__(self): + return _rebuild_exc, (self.exc, self.tb) + +def _rebuild_exc(exc: BaseException, tb: str): + """ + Reconstructs the exception, putting the original exception as cause. + """ + exc.__cause__ = RemoteException(tb) + return exc class WorkerDiedException(RuntimeError): """Raised if a worker process dies unexpected.""" + original_exception: Optional[BaseException] - def __init__(self, message, original_exception: BaseException = None, original_traceback: TracebackType = None): - super(WorkerDiedException, self).__init__(message) + def __init__(self, message: Optional[str], original_exception: Optional[BaseException] = None): + super().__init__(message) self.original_exception = original_exception - self.original_traceback = original_traceback def build_many(target, args, processes=None): @@ -94,6 +131,10 @@ def build_many(target, args, processes=None): sage: build_many(target, range(8), processes=8) Traceback (most recent call last): ... + raise ZeroDivisionError("rational division by zero") + ZeroDivisionError: rational division by zero + ... + raise worker_exc.original_exception ZeroDivisionError: rational division by zero Similarly, if one of the worker processes dies unexpectedly otherwise exits @@ -136,8 +177,8 @@ def build_many(target, args, processes=None): def run_worker(target, queue, idx, task): try: result = target(task) - except BaseException: - queue.put((None, sys.exc_info())) + except BaseException as exc: + queue.put((None, RemoteExceptionWrapper(exc))) else: queue.put((idx, result)) @@ -175,8 +216,8 @@ def bring_out_yer_dead(w, task, exitcode): if result[0] is None: # Indicates that an exception occurred in the target function - _, exception, tracback = result[1] - raise WorkerDiedException('', original_exception=exception, original_traceback=tracback) + exception = result[1] + raise WorkerDiedException('', original_exception=exception) else: results.append(result) @@ -291,8 +332,8 @@ def reap_workers(waited_pid=None, waited_exitcode=None): # Re-raise the RuntimeError from bring_out_yer_dead set if a # worker died unexpectedly, or the original exception if it's # wrapping one - if worker_exc.original_exception: - raise worker_exc.original_exception.with_traceback(worker_exc.original_traceback) + if worker_exc.original_exception is not None: + raise worker_exc.original_exception else: raise worker_exc From 37f1d0c8011322058e1e6881d8ebd46446b700e0 Mon Sep 17 00:00:00 2001 From: Tobias Diez Date: Sat, 4 Dec 2021 19:58:21 +0100 Subject: [PATCH 3/5] Move _rebuild_exc and cleanup code --- src/sage_docbuild/utils.py | 35 +++++++++++++++++++++++------------ 1 file changed, 23 insertions(+), 12 deletions(-) diff --git a/src/sage_docbuild/utils.py b/src/sage_docbuild/utils.py index d3ac608a8ba..bccda4796e3 100644 --- a/src/sage_docbuild/utils.py +++ b/src/sage_docbuild/utils.py @@ -5,49 +5,58 @@ import traceback from typing import Optional + class RemoteException(Exception): """ Raised if an exception occurred in one of the child processes. """ + tb: str + def __init__(self, tb: str): self.tb = tb def __str__(self): return self.tb + class RemoteExceptionWrapper: """ - Used by child processes to capture exceptions thrown during execution and + Used by child processes to capture exceptions thrown during execution and report them to the main process, including the correct traceback. """ + exc: BaseException tb: str def __init__(self, exc: BaseException): # We cannot pickle the traceback, thus convert it to a string. # Later on unpickling, we set the original tracback as the cause of the exception - # This approach is taken from https://bugs.python.org/issue13831 + # This approach is taken from https://bugs.python.org/issue13831 tb = traceback.format_exception(type(exc), exc, exc.__traceback__) - tb = ''.join(tb) + tb = "".join(tb) self.exc = exc self.tb = f'\n"""\n{tb}"""' def __reduce__(self): + def _rebuild_exc(exc: BaseException, tb: str): + """ + Reconstructs the exception, putting the original exception as cause. + """ + exc.__cause__ = RemoteException(tb) + return exc + return _rebuild_exc, (self.exc, self.tb) -def _rebuild_exc(exc: BaseException, tb: str): - """ - Reconstructs the exception, putting the original exception as cause. - """ - exc.__cause__ = RemoteException(tb) - return exc class WorkerDiedException(RuntimeError): """Raised if a worker process dies unexpected.""" + original_exception: Optional[BaseException] - def __init__(self, message: Optional[str], original_exception: Optional[BaseException] = None): + def __init__( + self, message: Optional[str], original_exception: Optional[BaseException] = None + ): super().__init__(message) self.original_exception = original_exception @@ -197,7 +206,9 @@ def bring_out_yer_dead(w, task, exitcode): w._popen.returncode = exitcode if exitcode != 0: - raise WorkerDiedException(f"worker for {task[1]} died with non-zero exit code {w.exitcode}") + raise WorkerDiedException( + f"worker for {task[1]} died with non-zero exit code {w.exitcode}" + ) # Get result from the queue; depending on ordering this may not be # *the* result for this worker, but for each completed worker there @@ -216,7 +227,7 @@ def bring_out_yer_dead(w, task, exitcode): if result[0] is None: # Indicates that an exception occurred in the target function exception = result[1] - raise WorkerDiedException('', original_exception=exception) + raise WorkerDiedException("", original_exception=exception) else: results.append(result) From 45ef9e34f138ce14c96646d7e5a31d5455d19b82 Mon Sep 17 00:00:00 2001 From: Tobias Diez Date: Mon, 6 Dec 2021 21:35:08 +0100 Subject: [PATCH 4/5] Fix code and add doctest --- src/sage_docbuild/utils.py | 49 ++++++++++++++++++++------------------ 1 file changed, 26 insertions(+), 23 deletions(-) diff --git a/src/sage_docbuild/utils.py b/src/sage_docbuild/utils.py index bccda4796e3..7242c4a5442 100644 --- a/src/sage_docbuild/utils.py +++ b/src/sage_docbuild/utils.py @@ -38,15 +38,23 @@ def __init__(self, exc: BaseException): self.exc = exc self.tb = f'\n"""\n{tb}"""' - def __reduce__(self): - def _rebuild_exc(exc: BaseException, tb: str): - """ - Reconstructs the exception, putting the original exception as cause. - """ - exc.__cause__ = RemoteException(tb) - return exc + @staticmethod + def _rebuild_exc(exc: BaseException, tb: str): + """ + Reconstructs the exception, putting the original exception as cause. + """ + exc.__cause__ = RemoteException(tb) + return exc - return _rebuild_exc, (self.exc, self.tb) + def __reduce__(self): + """ + TESTS:: + sage: import pickle + sage: from sage_docbuild.utils import RemoteExceptionWrapper + sage: pickle.dumps(RemoteExceptionWrapper(ZeroDivisionError()), 0).decode() + ...RemoteExceptionWrapper...ZeroDivisionError... + """ + return RemoteExceptionWrapper._rebuild_exc, (self.exc, self.tb) class WorkerDiedException(RuntimeError): @@ -166,11 +174,12 @@ def build_many(target, args, processes=None): WorkerDiedException: worker for 4 died with non-zero exit code -9 """ from multiprocessing import Process, Queue, cpu_count, set_start_method + # With OS X, Python 3.8 defaults to use 'spawn' instead of 'fork' # in multiprocessing, and Sage docbuilding doesn't work with # 'spawn'. See trac #27754. - if os.uname().sysname == 'Darwin': - set_start_method('fork', force=True) + if os.uname().sysname == "Darwin": + set_start_method("fork", force=True) from queue import Empty if processes is None: @@ -213,24 +222,19 @@ def bring_out_yer_dead(w, task, exitcode): # Get result from the queue; depending on ordering this may not be # *the* result for this worker, but for each completed worker there # should be *a* result so let's get it - result = None try: result = result_queue.get_nowait() + if result[0] is None: + # Indicates that an exception occurred in the target function + exception = result[1] + raise WorkerDiedException("", original_exception=exception) + else: + results.append(result) except Empty: # Generally shouldn't happen but could in case of a race condition; # don't worry we'll collect any remaining results at the end. pass - if result is None: - return None - - if result[0] is None: - # Indicates that an exception occurred in the target function - exception = result[1] - raise WorkerDiedException("", original_exception=exception) - else: - results.append(result) - # Helps multiprocessing with some internal bookkeeping w.join() @@ -285,8 +289,7 @@ def reap_workers(waited_pid=None, waited_exitcode=None): except StopIteration: pass else: - w = Process(target=run_worker, - args=((target, result_queue) + task)) + w = Process(target=run_worker, args=((target, result_queue) + task)) w.start() # Pair the new worker with the task it's performing (mostly # for debugging purposes) From c37d055673bb573423a3200a6a715db567de436f Mon Sep 17 00:00:00 2001 From: Tobias Diez Date: Wed, 15 Dec 2021 21:51:16 +0100 Subject: [PATCH 5/5] Add docstrings --- src/sage_docbuild/utils.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/src/sage_docbuild/utils.py b/src/sage_docbuild/utils.py index 7242c4a5442..e6026120d04 100644 --- a/src/sage_docbuild/utils.py +++ b/src/sage_docbuild/utils.py @@ -14,9 +14,19 @@ class RemoteException(Exception): tb: str def __init__(self, tb: str): + """ + Initialize the exception. + + INPUT: + + - ``tb`` -- the traceback of the exception. + """ self.tb = tb def __str__(self): + """ + Return a string representation of the exception. + """ return self.tb @@ -30,6 +40,15 @@ class RemoteExceptionWrapper: tb: str def __init__(self, exc: BaseException): + """ + Initialize the exception wrapper. + + INPUT: + + - ``exc`` -- the exception to wrap. + """ + self.exc = exc + self.tb = traceback.format_exc() # We cannot pickle the traceback, thus convert it to a string. # Later on unpickling, we set the original tracback as the cause of the exception # This approach is taken from https://bugs.python.org/issue13831