pyclass: fix reference count issue in subclass new #1365
Conversation
|
Hmm the tests ran fine on my local Python 3.9 and 3.6, I'll take another look tomorrow... |
davidhewitt
force-pushed
the
fix-tp-dealloc
branch
from
1fdcf2e
to
a1ef01a
Compare
davidhewitt
force-pushed
the
fix-tp-dealloc
branch
from
210d1f9
to
5fa9b72
Compare
|
@kngwyu I think this is ready for another look! |
src/pyclass.rs
Outdated
|
|
||
| // Must check version at runtime because of abi3 wheels which could run against a higher | ||
| // version than the config. | ||
| static IS_NOT_PYTHON_3_8: GILOnceCell<bool> = GILOnceCell::new(); |
There was a problem hiding this comment.
How about checking the current refcount instead of the version?
There was a problem hiding this comment.
I couldn't find a way to detect this case using the type reference count - it increases by one on all versions I tested.
I guess I need to make this check only with Py_Limited_API, so I could avoid checking the version in more cases.
|
I pushed a new version of this PR which only does the runtime version check if Let me know if there's any other changes you would like to this. Once you're happy, I'll merge this and release 0.13.1 after. |
davidhewitt
force-pushed
the
fix-tp-dealloc
branch
3 times, most recently
from
b080946
to
f6077d2
Compare
davidhewitt
force-pushed
the
fix-tp-dealloc
branch
from
f6077d2
to
b92441c
Compare
|
LGTM, thanks! |
This fixes a missing reference count increase (to work around a cpython bug) on Python versions 3.7 and older - see #1363 (comment)
It's a fairly nasty bug which leads to pretty immediate weird behaviour / crashes when tripped, so I think it is worth releasing 0.13.1 once this is merged.
Closes #1363