[issue23571] Raise SystemError if a function returns a result with an exception set

Changes by Jesús Cea Avión : -- nosy: +jcea ___ Python tracker ___ ___ Python-bugs-list mailing

[issue23571] Raise SystemError if a function returns a result with an exception set

STINNER Victor added the comment: I close the issue. -- status: open - closed ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23571 ___ ___

[issue23571] Raise SystemError if a function returns a result with an exception set

STINNER Victor added the comment: Serhiy Storchaka added the comment: The second (exception == NULL) check in _Py_PrintFatalError() looks suspicious. When it is possible? And if it is possible, can it cause leaks? Sorry, I have no idea. I didn't write this code myself. It comes from

[issue23571] Raise SystemError if a function returns a result with an exception set

Serhiy Storchaka added the comment: The second (exception == NULL) check in _Py_PrintFatalError() looks suspicious. When it is possible? And if it is possible, can it cause leaks? -- ___ Python tracker rep...@bugs.python.org

[issue23571] Raise SystemError if a function returns a result with an exception set

STINNER Victor added the comment: That last commit fixed compatibility with Django. Good. Instead of an assert(), you could use Py_FatalError() at the end of _Py_CheckFunctionResult(). _Py_CheckFunctionResult() now calls Py_FatalError() in debug mode. By the way, I fixed various issues

[issue23571] Raise SystemError if a function returns a result with an exception set

Roundup Robot added the comment: New changeset efb2c9ac2f88 by Victor Stinner in branch '3.4': Issue #23571: Enhance Py_FatalError() https://hg.python.org/cpython/rev/efb2c9ac2f88 New changeset 6303795f035a by Victor Stinner in branch 'default': (Merge 3.4) Issue #23571: Enhance Py_FatalError()

[issue23571] Raise SystemError if a function returns a result with an exception set

Roundup Robot added the comment: New changeset 850b9dcd0534 by Victor Stinner in branch 'default': Issue #23571: In debug mode, _Py_CheckFunctionResult() now calls https://hg.python.org/cpython/rev/850b9dcd0534 New changeset da252f12352a by Victor Stinner in branch '3.4': Issue #23571:

[issue23571] Raise SystemError if a function returns a result with an exception set

Tim Graham added the comment: That last commit fixed compatibility with Django. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23571 ___ ___

[issue23571] Raise SystemError if a function returns a result with an exception set

Roundup Robot added the comment: New changeset 2e14ca478a57 by Victor Stinner in branch 'default': Issue #23571: PyErr_FormatV() and PyErr_SetObject() now always clear the https://hg.python.org/cpython/rev/2e14ca478a57 -- ___ Python tracker

[issue23571] Raise SystemError if a function returns a result with an exception set

Roundup Robot added the comment: New changeset 2a18b958f1e1 by Victor Stinner in branch 'default': Issue #23571: Enhance _Py_CheckFunctionResult() https://hg.python.org/cpython/rev/2a18b958f1e1 -- ___ Python tracker rep...@bugs.python.org

[issue23571] Raise SystemError if a function returns a result with an exception set

Roundup Robot added the comment: New changeset 004e3870d9e6 by Victor Stinner in branch '3.4': Issue #23571: If io.TextIOWrapper constructor fails in _Py_DisplaySourceLine(), https://hg.python.org/cpython/rev/004e3870d9e6 -- ___ Python tracker

[issue23571] Raise SystemError if a function returns a result with an exception set

Roundup Robot added the comment: New changeset 1c2376825dd2 by Victor Stinner in branch '3.4': Issue #23571: Fix reentrant call to Py_FatalError() https://hg.python.org/cpython/rev/1c2376825dd2 New changeset e9ba95418af8 by Victor Stinner in branch 'default': (Merge 3.4) Issue #23571: Fix

[issue23571] Raise SystemError if a function returns a result with an exception set

Serhiy Storchaka added the comment: Minimal reproducer: try: ... 1/0 ... except: ... int('spam') ... Traceback (most recent call last): File stdin, line 2, in module ZeroDivisionError: division by zero During handling of the above exception, another exception occurred: ValueError:

[issue23571] Raise SystemError if a function returns a result with an exception set

STINNER Victor added the comment: Serhiy Storchaka added the comment: Why this assert is needed? Why not always raise SystemError? A SystemError exception may be ignored by a generic except Exception: pass or logged at debug level at then ignored. I consider that the bug is important, and

[issue23571] Raise SystemError if a function returns a result with an exception set

Roundup Robot added the comment: New changeset 970f33dff5ca by Victor Stinner in branch 'default': Issue #23571: Fix test_capi https://hg.python.org/cpython/rev/970f33dff5ca -- ___ Python tracker rep...@bugs.python.org

[issue23571] Raise SystemError if a function returns a result with an exception set

Serhiy Storchaka added the comment: This exception or assertion is triggered by the bug in CPython or in an extension. CPython developer uses release and debug builds of CPython and can get both exception or assertion. An extension developer usually uses release build of CPython with release

[issue23571] Raise SystemError if a function returns a result with an exception set

STINNER Victor added the comment: Serhiy was right: we should mention the name of the function in the exception. In the Django traceback, it's not easy to guess what raised the SystemError. Ok to mention the change in What's New in Python 3.5. --

[issue23571] Raise SystemError if a function returns a result with an exception set

Antoine Pitrou added the comment: Instead of an assert(), you could use Py_FatalError() at the end of _Py_CheckFunctionResult(). -- nosy: +pitrou ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23571

[issue23571] Raise SystemError if a function returns a result with an exception set

Serhiy Storchaka added the comment: Non-Linux buildbots failed. http://buildbot.python.org/all/builders/AMD64%20Snow%20Leop%203.x/builds/2798/steps/test/logs/stdio == FAIL: test_return_result_with_error

[issue23571] Raise SystemError if a function returns a result with an exception set

STINNER Victor added the comment: I believe that this patch exposes some subtle bugs in Django (see https://gist.github.com/berkerpeksag/8b8dbe594eb1a1c51275) and it would be great to add a note for third party libraries. Berker, can you please rerun your test with my new commit to check

[issue23571] Raise SystemError if a function returns a result with an exception set

Roundup Robot added the comment: New changeset f30a5f6a665c by Victor Stinner in branch 'default': Issue #23571: _Py_CheckFunctionResult() now gives the name of the function https://hg.python.org/cpython/rev/f30a5f6a665c -- ___ Python tracker

[issue23571] Raise SystemError if a function returns a result with an exception set

STINNER Victor added the comment: Le 21 mars 2015 18:05, Serhiy Storchaka rep...@bugs.python.org a écrit : But an assertion itself provides less information than an exception. Debug build is less informative than release build. I like Antoine's idea to replace the assertion with

[issue23571] Raise SystemError if a function returns a result with an exception set

Tim Graham added the comment: Here's an exception in Django after the latest patch. The Django code block in the last exception catches ValueError, but this doesn't seem to work any longer since it's wrapped in SystemError. As Berker mentioned, some upgrade tips would be great as I'm not sure

[issue23571] Raise SystemError if a function returns a result with an exception set

Berker Peksag added the comment: Can we add a note (probably with an example) to the Porting to Python 3.5 section of https://docs.python.org/3.5/whatsnew/3.5.html#porting-to-python-3-5 I believe that this patch exposes some subtle bugs in Django (see

[issue23571] Raise SystemError if a function returns a result with an exception set

Roundup Robot added the comment: New changeset 91fbe0fff882 by Victor Stinner in branch 'default': Issue #23571: Restore removed assert(!PyErr_Occurred()); in https://hg.python.org/cpython/rev/91fbe0fff882 -- ___ Python tracker rep...@bugs.python.org

[issue23571] Raise SystemError if a function returns a result with an exception set

Changes by Berker Peksag berker.pek...@gmail.com: -- stage: commit review - resolved ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23571 ___ ___

[issue23571] Raise SystemError if a function returns a result with an exception set

Roundup Robot added the comment: New changeset 52c7017fdcdd by Victor Stinner in branch 'default': Issue #23571: Oops, fix #ifdef assert() https://hg.python.org/cpython/rev/52c7017fdcdd -- ___ Python tracker rep...@bugs.python.org

[issue23571] Raise SystemError if a function returns a result with an exception set

STINNER Victor added the comment: Thanks for the very useful reviews Serhiy! -- resolution: - fixed status: open - closed ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23571 ___

[issue23571] Raise SystemError if a function returns a result with an exception set

Roundup Robot added the comment: New changeset 97ef38236dc1 by Victor Stinner in branch 'default': Issue #23571: PyObject_Call(), PyCFunction_Call() and call_function() now https://hg.python.org/cpython/rev/97ef38236dc1 -- nosy: +python-dev ___

[issue23571] Raise SystemError if a function returns a result with an exception set

STINNER Victor added the comment: Updated patch to take in account more Serhiy's comments. -- Added file: http://bugs.python.org/file38358/check_result-4.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23571

[issue23571] Raise SystemError if a function returns a result with an exception set

STINNER Victor added the comment: Oh, test_io and test_sqlite are failing in release mode with check_result-4.patch. io and sqlite modules only call PyErr_Clear() in debug mode. It's my fault, I added them, but I made them conditionnal to not impact performances. It's now fixed in the new

[issue23571] Raise SystemError if a function returns a result with an exception set

Serhiy Storchaka added the comment: Could you please run microbenchmarks from issue23507? -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23571 ___

[issue23571] Raise SystemError if a function returns a result with an exception set

STINNER Victor added the comment: I ran pybench, even if I know that it's not really revelant: overall 0.3% slowdown. But pybench doesn't look reliable: some tests are faster, which looks like noise in the benchmark.

[issue23571] Raise SystemError if a function returns a result with an exception set

STINNER Victor added the comment: New patch to take in account Serhiy's comments. I also fixed call_function(), I forgot to call _Py_CheckFunctionResult() in two cases. -- Added file: http://bugs.python.org/file38357/check_result-3.patch ___ Python

[issue23571] Raise SystemError if a function returns a result with an exception set

STINNER Victor added the comment: I ran http://hg.python.org/benchmarks/ Result: --- $ python3 perf.py -r -b default ~/prog/python/default/python.orig ~/prog/python/default/python.patched INFO:root:Automatically selected timer: perf_counter (...) Report on Linux smithers

[issue23571] Raise SystemError if a function returns a result with an exception set

STINNER Victor added the comment: Even worse benchmark: timeit :-) python -m timeit -s 'def f(): pass' 'f()' * original: 67.5 nanoseconds * patched: 64.1 nanoseconds (faster!?) -- ___ Python tracker rep...@bugs.python.org

[issue23571] Raise SystemError if a function returns a result with an exception set

STINNER Victor added the comment: New patch which removes some assertions and move other assertions on result/PyErr_Occurred. I removed useless duplicated assertions and move the final assertion at the end of PyEval_EvalFrameEx() to guarantee that the function behave correctly. --

[issue23571] Raise SystemError if a function returns a result with an exception set

Serhiy Storchaka added the comment: I'm not sure that all removed assertions are not needed. There are many ways to get raised exception, not only by calling a function. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23571

[issue23571] Raise SystemError if a function returns a result with an exception set

Serhiy Storchaka added the comment: We can ignore such little slow down (if this is not just compiler artifact). The patch LGTM. I afraid there will be issues with third-party code that will need an adaptation to 3.5, but on other hand, this patch could help with catching some hidden bugs.

[issue23571] Raise SystemError if a function returns a result with an exception set

STINNER Victor added the comment: 2015-03-06 16:55 GMT+01:00 Serhiy Storchaka rep...@bugs.python.org: We can ignore such little slow down (if this is not just compiler artifact). Maybe it's time to play with micro-optimizations like __builtin_expect :-) But I would prefer to do that in a

[issue23571] Raise SystemError if a function returns a result with an exception set

Changes by Serhiy Storchaka storch...@gmail.com: -- assignee: - haypo stage: - commit review ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23571 ___

[issue23571] Raise SystemError if a function returns a result with an exception set

Serhiy Storchaka added the comment: If the patch has a significant overhead: _Py_CheckFunctionResult() may be marked to be inlined, and PyCFunction_Call() and PyObject_Call() checks may be marked as unlikely using GCC __builtin_expect(), something like: Could you please open separate issue

[issue23571] Raise SystemError if a function returns a result with an exception set

STINNER Victor added the comment: 2015-03-03 13:49 GMT+01:00 Serhiy Storchaka rep...@bugs.python.org: If the patch has a significant overhead: _Py_CheckFunctionResult() may be marked to be inlined, and PyCFunction_Call() and PyObject_Call() checks may be marked as unlikely using GCC

[issue23571] Raise SystemError if a function returns a result with an exception set

New submission from STINNER Victor: Attached patch changes PyObject_Call() and PyCFunction_Call() to raise a SystemError and destroy the result (Py_DECREF) if a function returns a result with an exception set. I also refactored PyCFunction_Call() and added assert(!PyErr_Occurred()); at the

[issue23571] Raise SystemError if a function returns a result with an exception set

Changes by STINNER Victor victor.stin...@gmail.com: -- keywords: +patch Added file: http://bugs.python.org/file38312/check_result.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23571