msg270713 - (view) |
Author: Romuald Brunet (Romuald) * |
Date: 2016-07-18 06:26 |
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
|
msg270728 - (view) |
Author: Antti Haapala (ztane) * |
Date: 2016-07-18 08:51 |
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
PyErr_SetString(PyExc_RuntimeError,
"No active exception to reraise");
What happens is that PyThreadState is initialized to *all* NULL pointers on the new thread on multiprocessing, however `type` is expected to point to `Py_None` when no exception has been raised:
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
|
msg270729 - (view) |
Author: Antti Haapala (ztane) * |
Date: 2016-07-18 09:06 |
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)
tstate->exc_type != NULL ? tstate->exc_type : Py_None,
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.
|
msg270730 - (view) |
Author: Antti Haapala (ztane) * |
Date: 2016-07-18 09:07 |
more easily reproducible by
import threading
def foo():
raise
threading.Thread(target=foo).start()
|
msg270733 - (view) |
Author: Xiang Zhang (xiang.zhang) *  |
Date: 2016-07-18 09:13 |
It seems the thread state is initialized in thread_PyThread_start_new_thread. It calls _PyThreadState_Prealloc which initialize exc_type to NULL.
|
msg270734 - (view) |
Author: Xiang Zhang (xiang.zhang) *  |
Date: 2016-07-18 09:21 |
I am curious why individual `raise` doesn't get the same problem in single thread program.
|
msg270735 - (view) |
Author: Antti Haapala (ztane) * |
Date: 2016-07-18 09:25 |
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.
|
msg270746 - (view) |
Author: Xiang Zhang (xiang.zhang) *  |
Date: 2016-07-18 10:41 |
> 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.
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.
|
msg270755 - (view) |
Author: Xiang Zhang (xiang.zhang) *  |
Date: 2016-07-18 13:38 |
Upload a patch.
|
msg270757 - (view) |
Author: Berker Peksag (berker.peksag) *  |
Date: 2016-07-18 14:06 |
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.
|
msg270759 - (view) |
Author: Xiang Zhang (xiang.zhang) *  |
Date: 2016-07-18 14:36 |
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.
|
msg270761 - (view) |
Author: R. David Murray (r.david.murray) *  |
Date: 2016-07-18 14:52 |
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.
|
msg270763 - (view) |
Author: Xiang Zhang (xiang.zhang) *  |
Date: 2016-07-18 15:30 |
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:
import some_module
except ImportError:
some_module = ...
So I think after startup the variable can't be NULL. But we do see a NULL case here.
|
msg271802 - (view) |
Author: Xiang Zhang (xiang.zhang) *  |
Date: 2016-08-02 04:48 |
Update the patch with Berker's suggestion: add test in test_threading instead of test_raise. Hope someone is willing to merge this.
|
msg271822 - (view) |
Author: Berker Peksag (berker.peksag) *  |
Date: 2016-08-02 13:51 |
The test code in issue27558_v2.patch looks OK to me.
|
msg272407 - (view) |
Author: Xiang Zhang (xiang.zhang) *  |
Date: 2016-08-11 07:13 |
Ping. Mind to merge?
|
msg272409 - (view) |
Author: Raymond Hettinger (rhettinger) *  |
Date: 2016-08-11 07:25 |
Davin, would you like to finish this one (NEWS and ACKS entries) and get it committed?
|
msg272436 - (view) |
Author: Berker Peksag (berker.peksag) *  |
Date: 2016-08-11 10:32 |
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.
|
msg272938 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2016-08-17 13:25 |
I reviewed issue27558_v2.patch, see my comments.
|
msg272943 - (view) |
Author: Xiang Zhang (xiang.zhang) *  |
Date: 2016-08-17 13:57 |
Thanks for your review, Victor. :) Leave a reply.
|
msg273011 - (view) |
Author: Xiang Zhang (xiang.zhang) *  |
Date: 2016-08-18 04:14 |
Victor, upload a new patch, changing the test case to the approach you prefer. ;)
|
msg273043 - (view) |
Author: Roundup Robot (python-dev)  |
Date: 2016-08-18 16:15 |
New changeset ba45fbb16499 by Victor Stinner in branch '3.5':
Fix SystemError in "raise" statement
https://hg.python.org/cpython/rev/ba45fbb16499
|
msg273044 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2016-08-18 16:19 |
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.
|
msg273046 - (view) |
Author: Xiang Zhang (xiang.zhang) *  |
Date: 2016-08-18 16:23 |
Thanks for your work too! ;) I agree to close.
|
|
Date |
User |
Action |
Args |
2022-04-11 14:58:33 | admin | set | github: 71745 |
2016-08-19 07:20:26 | SilentGhost | set | stage: patch review -> resolved |
2016-08-18 16:24:42 | vstinner | set | status: open -> closed resolution: fixed |
2016-08-18 16:23:55 | xiang.zhang | set | messages:
+ msg273046 |
2016-08-18 16:19:18 | vstinner | set | messages:
+ msg273044 |
2016-08-18 16:15:15 | python-dev | set | nosy:
+ python-dev messages:
+ msg273043
|
2016-08-18 04:14:23 | xiang.zhang | set | files:
+ issue27558_v3.patch
messages:
+ msg273011 |
2016-08-17 13:57:55 | xiang.zhang | set | messages:
+ msg272943 |
2016-08-17 13:25:48 | vstinner | set | messages:
+ msg272938 |
2016-08-11 10:32:22 | berker.peksag | set | messages:
+ msg272436 |
2016-08-11 07:25:36 | rhettinger | set | assignee: davin
messages:
+ msg272409 nosy:
+ rhettinger |
2016-08-11 07:13:17 | xiang.zhang | set | messages:
+ msg272407 |
2016-08-02 13:51:17 | berker.peksag | set | messages:
+ msg271822 |
2016-08-02 04:48:29 | xiang.zhang | set | files:
+ issue27558_v2.patch
messages:
+ msg271802 |
2016-07-18 15:30:02 | xiang.zhang | set | messages:
+ msg270763 |
2016-07-18 14:52:00 | r.david.murray | set | nosy:
+ r.david.murray messages:
+ msg270761
|
2016-07-18 14:36:43 | xiang.zhang | set | messages:
+ msg270759 |
2016-07-18 14:28:47 | vstinner | set | nosy:
+ vstinner
|
2016-07-18 14:06:20 | berker.peksag | set | versions:
- Python 3.4 nosy:
+ berker.peksag
messages:
+ msg270757
stage: needs patch -> patch review |
2016-07-18 13:38:26 | xiang.zhang | set | files:
+ issue27558.patch keywords:
+ patch messages:
+ msg270755
|
2016-07-18 11:06:02 | abarry | set | nosy:
+ pitrou, davin, abarry title: SystemError inside multiprocessing.dummy Pool.map -> SystemError with bare `raise` in threading or multiprocessing
stage: needs patch versions:
+ Python 3.6, - Python 3.2, Python 3.3 |
2016-07-18 10:41:34 | xiang.zhang | set | messages:
+ msg270746 |
2016-07-18 09:25:18 | ztane | set | messages:
+ msg270735 |
2016-07-18 09:21:15 | xiang.zhang | set | messages:
+ msg270734 |
2016-07-18 09:13:56 | xiang.zhang | set | messages:
+ msg270733 |
2016-07-18 09:07:47 | ztane | set | messages:
+ msg270730 |
2016-07-18 09:06:24 | ztane | set | messages:
+ msg270729 |
2016-07-18 08:51:34 | ztane | set | nosy:
+ ztane messages:
+ msg270728
|
2016-07-18 06:35:32 | xiang.zhang | set | nosy:
+ xiang.zhang
|
2016-07-18 06:26:43 | Romuald | create | |