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

BUG: Fix reference count issues #3022

Merged
merged 3 commits into from Jun 29, 2019
Merged

BUG: Fix reference count issues #3022

merged 3 commits into from Jun 29, 2019

Conversation

seberg
Copy link
Contributor

@seberg seberg commented Jun 28, 2019

The new PyInt_FromSsize_t reference was not decref'ed in general.


I was hacking on numpy using pytest-leaks and ran into a reference issues within cython (numpy.random). This is the on that I managed to track down for now. There is another coming up (although most likely I will only open an issue).

I suppose these should get tests? Just ping me, and I will look into it.

@seberg seberg force-pushed the getitem_refcnt branch 3 times, most recently from 43d5b85 to 4f50113 Compare June 28, 2019 23:30
The new PyInt_FromSsize_t reference was not decref'ed in general.
@seberg
Copy link
Contributor Author

seberg commented Jun 28, 2019

Sorry for the many forced pushs (did not run tests, which I guess would have told me that my code looked weird) and adding of more commits as I am on to a few other things. If you want me to split it up or do style fixes, etc. I am happy to do that. Until then, may push more small things as I track them down.

@seberg seberg changed the title BUG: Fix reference count issue in __Pyx_(Get|Set)ItemInt_Fast BUG: Fix reference count issues Jun 29, 2019
@seberg
Copy link
Contributor Author

seberg commented Jun 29, 2019

OK, this together with gh-3023 makes the numpy random tests run through cleanly with respect to refcounting issues, so for me this has settled already.

Copy link
Contributor

@scoder scoder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catches.

@@ -497,8 +500,12 @@ static CYTHON_INLINE int __Pyx_SetItemInt_Fast(PyObject *o, Py_ssize_t i, PyObje
PyMappingMethods *mm = Py_TYPE(o)->tp_as_mapping;
PySequenceMethods *sm = Py_TYPE(o)->tp_as_sequence;
if (mm && mm->mp_ass_subscript) {
int r
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Semicolon missing.

@@ -432,8 +432,11 @@ static CYTHON_INLINE PyObject *__Pyx_GetItemInt_Fast(PyObject *o, Py_ssize_t i,
PyMappingMethods *mm = Py_TYPE(o)->tp_as_mapping;
PySequenceMethods *sm = Py_TYPE(o)->tp_as_sequence;
if (mm && mm->mp_subscript) {
PyObject *key = PyInt_FromSsize_t(i);
return likely(key) ? mm->mp_subscript(o, key) : NULL;
PyObject r, *key = PyInt_FromSsize_t(i);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pointer-star missing.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Heh, thanks... worst fixup ever

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

Successfully merging this pull request may close these issues.

None yet

2 participants