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

Pynac should check for errors when calling PyObject_RichCompareBool() #24522

Closed
embray opened this issue Jan 11, 2018 · 8 comments
Closed

Pynac should check for errors when calling PyObject_RichCompareBool() #24522

embray opened this issue Jan 11, 2018 · 8 comments

Comments

@embray
Copy link
Contributor

embray commented Jan 11, 2018

There are a number of methods in pynac's ginac/numeric.cpp module that don't check for error results from PyObject_RichCompareBool (which returns -1 on error).

This leads to unhandled exceptions that, on Python 2, were somewhat by luck being cleared via PyObject_Clear() later in the call stack.

On Python 3, the interpreter asserts PyErr_Occurred in more places and we don't get so lucky.

Upstream PR: pynac/pynac#297

Upstream: Fixed upstream, in a later stable release.

CC: @rwst

Component: packages: standard

Keywords: pynac

Issue created by migration from https://trac.sagemath.org/ticket/24522

@embray embray added this to the sage-8.2 milestone Jan 11, 2018
@embray
Copy link
Contributor Author

embray commented Jan 12, 2018

comment:2

Yes, this still looks like what I described. On Python 3 the exception leaks out, and slightly later on a call to PyObject_IsInstance in turn calls the type's __instancecheck__ which goes through _PyObject_FastCallDict which in turn asserts PyErr_Occurred.

On Python 3 there is no _PyObject_FastCallDict, no assertion checks on PyErr_Occurred in the relevant code path, and eventually that error gets cleared.

@jdemeyer

This comment has been minimized.

@jdemeyer jdemeyer changed the title py3: bug in pynac numeric.cpp with PyObject_RichCompareBool Pynac should check for errors when calling PyObject_RichCompareBool() Jan 12, 2018
@jdemeyer jdemeyer self-assigned this Jan 12, 2018
@embray
Copy link
Contributor Author

embray commented Jan 12, 2018

comment:4

I have an upstream fix for this ready--just need to make a PR...

@embray
Copy link
Contributor Author

embray commented Jan 15, 2018

Upstream: Reported upstream. No feedback yet.

@embray

This comment has been minimized.

@rwst
Copy link

rwst commented Jan 15, 2018

Changed upstream from Reported upstream. No feedback yet. to Fixed upstream, in a later stable release.

@embray
Copy link
Contributor Author

embray commented Jan 15, 2018

comment:7

Thanks! This is low priority for now, as it only affects a few things on Python 3. We can close this along with the next update to pynac in Sage.

@fchapoton
Copy link
Contributor

comment:8

Can we close this now ?

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

4 participants