Skip to content
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

except dynamic_tuple: fails to catch on Python 3 #2425

Closed
jamadden opened this issue Jun 26, 2018 · 3 comments
Closed

except dynamic_tuple: fails to catch on Python 3 #2425

jamadden opened this issue Jun 26, 2018 · 3 comments

Comments

@jamadden
Copy link
Contributor

Given this code in foo.py:

from __future__ import print_function
to_catch = (AssertionError,)
try:
	raise AssertionError("AssertionError")
except to_catch as e:
	print("Caught", e)
except:
	print("Uncaught!")

Both Python 2 and Python 3 will print 'Caught':

$ python2.7 foo.py
Caught AssertionError
$ python3.6 foo.py
Caught AssertionError

But if we compile foo.py with Cython 0.28.3 and then invoke it, only Python 2 correctly catches the exception:

$ cat setup.py
from setuptools import setup
from Cython.Build import cythonize
from setuptools import Extension

setup(
	name='foo',
	ext_modules=cythonize('foo.py')
)
$ cat bar.py
import foo
$ python2.7 setup.py build_ext -i
...
$ python3.6 setup.py build_ext -i
...
$ mv foo.py foo.py.off # make sure we get the extension
$ python2.7 bar.py
Caught AssertionError
$ python3.6 bar.py
Uncaught!

The catch line compiles into a call to __Pyx_PyErr_GivenExceptionMatches

    /* "foo.py":5
 * try:
 * 	raise AssertionError("AssertionError")
 * except to_catch as e:             # <<<<<<<<<<<<<<
 * 	print("Caught", e)
 * except:
 */
    __Pyx_ErrFetch(&__pyx_t_4, &__pyx_t_5, &__pyx_t_6);
    __pyx_t_7 = __Pyx_GetModuleGlobalName(__pyx_n_s_to_catch); if (unlikely(!__pyx_t_7)) __PYX_ERR(0, 5, __pyx_L4_except_error)
    __Pyx_GOTREF(__pyx_t_7);
    __pyx_t_8 = __Pyx_PyErr_GivenExceptionMatches(__pyx_t_4, __pyx_t_7);
static CYTHON_INLINE int __Pyx_PyErr_GivenExceptionMatches(PyObject *err, PyObject* exc_type) {
    if (likely(err == exc_type)) return 1;
    if (likely(PyExceptionClass_Check(err))) {
        return __Pyx_inner_PyErr_GivenExceptionMatches2(err, NULL, exc_type);
    }
    return PyErr_GivenExceptionMatches(err, exc_type);
}

The definitions of __Pyx_inner_PyErr_GivenExceptionMatches2 are different on Python 2 and 3. On Python 2, it goes directly to PyObject_IsSubclass(err, exc_type), which correctly handles tuples.

On Python 3, though, it calls __Pyx_IsSubtype((PyTypeObject*)err, (PyTypeObject*)exc_type2);

static CYTHON_INLINE int __Pyx_inner_PyErr_GivenExceptionMatches2(PyObject *err, PyObject* exc_type1, PyObject *exc_type2) {
    int res = exc_type1 ? __Pyx_IsSubtype((PyTypeObject*)err, (PyTypeObject*)exc_type1) : 0;
    if (!res) {
        res = __Pyx_IsSubtype((PyTypeObject*)err, (PyTypeObject*)exc_type2);
    }
    return res;
}

That's an incorrect cast in the second parameter, and Pyx_IsSubtype doesn't correctly handle tuples for its second argument (luckily it doesn't crash!). If I modify this function to use PyObject_IsSubclass it works as expected.

jamadden added a commit to OpenNTI/nti.externalization that referenced this issue Jun 26, 2018
@mbuesch
Copy link
Contributor

mbuesch commented Jun 26, 2018

#2426 might be the same problem as this one.
I bisected the original breakage to 6cc0a3a.
For me, with current master it only fails to catch the exception, if the tuple is not a local.
So something between 6cc0a3a and master slightly improved the situation.

@scoder
Copy link
Contributor

scoder commented Jun 26, 2018

Thanks for the excellent analysis.

@jamadden
Copy link
Contributor Author

Thanks for the quick fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants