New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
SystemError with bare raise
in threading or multiprocessing
#71745
Comments
Raising without a previous exception inside a method called from multiprocessing.dummy.Pool.map will trigger a SystemError. SystemError: PyEval_EvalFrameEx returned NULL without setting an error Traceback (most recent call last):
File "example.py", line 16, in <module>
main()
File "example.py", line 11, in main
result = pool.map(test, [1])
File "/usr/local/Cellar/python3/3.5.2/Frameworks/Python.framework/Versions/3.5/lib/python3.5/multiprocessing/pool.py", line 260, in map
return self._map_async(func, iterable, mapstar, chunksize).get()
File "/usr/local/Cellar/python3/3.5.2/Frameworks/Python.framework/Versions/3.5/lib/python3.5/multiprocessing/pool.py", line 608, in get
raise self._value
File "/usr/local/Cellar/python3/3.5.2/Frameworks/Python.framework/Versions/3.5/lib/python3.5/multiprocessing/pool.py", line 119, in worker
result = (True, func(*args, **kwds))
File "/usr/local/Cellar/python3/3.5.2/Frameworks/Python.framework/Versions/3.5/lib/python3.5/multiprocessing/pool.py", line 44, in mapstar
return list(map(*args))
SystemError: PyEval_EvalFrameEx returned NULL without setting an error Reproducible example is attached |
Reproducible on Python 3.6a4ish on Ubuntu. I believe this needs forking multiprocessing. do_raise is called with 2 NULLs as arguments, it should raise
What happens is that PyThreadState is initialized to all NULL pointers on the new thread on multiprocessing, however PyThreadState *tstate = PyThreadState_GET();
PyObject *tb;
type = tstate->exc_type;
value = tstate->exc_value;
tb = tstate->exc_traceback;
if (type == Py_None) {
PyErr_SetString(PyExc_RuntimeError,
"No active exception to reraise");
return 0;
} I am not sure where the thread state should have been initialized though |
OTOH, if you put sys.exc_info() in place of raise there, it correctly (None, None, None) there, because it does (sysmodule.c:sys_exc_info)
Easiest fix would be to make do_raise test for both NULL and None, but I'd really consider fixing the new thread initialization if possible. |
more easily reproducible by import threading
def foo():
raise
threading.Thread(target=foo).start() |
It seems the thread state is initialized in thread_PyThread_start_new_thread. It calls _PyThreadState_Prealloc which initialize exc_type to NULL. |
I am curious why individual |
I was thinking that perhaps an exception is always raised somewhere before? I tried skipping site, but it still works, so I am not too sure. |
I think the easiest fix is OK. The new thread initialization is consistent and should not be changed. The Py_None seems to be set in somewhere in ceval but I am not sure where. You can see at https://hg.python.org/cpython/file/tip/Python/ceval.c#l1200 both NULL and Py_None are checked. |
raise
in threading or multiprocessing
Upload a patch. |
I would prefer Antti's test case in msg270730 since the bug can be reproduced by using threading module (you can use assert_python_ok() as a helper.) Lib/test_threading.py is probably a better place for the test. |
It's reasonable but I'd still like to hear others' voices since it looks more like to me it's the behaviour of raise itself rather than threading that crashes. |
Since ceval already has such a check, I think we should fix it by adding the check as proposed. Investigating whether the codebase could be simplified by making sure that Py_None is always there after startup would be a separate issue, I think, possibly related to PEP-432. |
A simple try ... except ... can set the variable to Py_None: >>> import threading
>>> def foo():
... try:
... raise RuntimeError
... except:
... pass
... raise
...
>>> threading.Thread(target=foo).start()
>>> Exception in thread Thread-1:
Traceback (most recent call last):
File "/usr/lib/python3.4/threading.py", line 920, in _bootstrap_inner
self.run()
File "/usr/lib/python3.4/threading.py", line 868, in run
self._target(*self._args, **self._kwargs)
File "<stdin>", line 6, in foo
RuntimeError: No active exception to reraise In the startup procedure, some c-apis are called and there are patterns like: try: So I think after startup the variable can't be NULL. But we do see a NULL case here. |
Update the patch with Berker's suggestion: add test in test_threading instead of test_raise. Hope someone is willing to merge this. |
The test code in issue27558_v2.patch looks OK to me. |
Ping. Mind to merge? |
Davin, would you like to finish this one (NEWS and ACKS entries) and get it committed? |
Raymond, please stop hijacking issues. This doesn't have anything to do with multiprocessing and there are already four core developers in the nosy list. If you really want to assign it to someone else, please at least wait for a month or ask if they have time to commit the patch first. Unless I'm missing something, this doesn't look like a release blocker to me and definitely doesn't look like some 6 years old ancient issue (the first message was posted three weeks ago) so there is no urgency here. |
I reviewed issue27558_v2.patch, see my comments. |
Thanks for your review, Victor. :) Leave a reply. |
Victor, upload a new patch, changing the test case to the approach you prefer. ;) |
New changeset ba45fbb16499 by Victor Stinner in branch '3.5': |
Thanks Xiang Zhang. The fix is obvious and simple, but it wasn't easy to identify it ;-) I pushed the fix to Python 3.5 and default (3.6). Python 2.7 doesn't crash, the bare "raise" statement raises the exception: TypeError('exceptions must be old-style classes or derived from BaseException, not NoneType',). I suggest to not touch Python 2.7 and close the issue. |
Thanks for your work too! ;) I agree to close. |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: