bpo-35961: Fix a crash in slice_richcompare()#11828
Conversation
Fix a crash in slice_richcompare(): ensure that the 2 temporary internal tuples used for the comparison are not tracked by the garbage collector, since they use borrowed references. The crash (or assertion error) occurred if a garbage collection occurred during slice_richcompare(), especially while calling PyObject_RichCompare(t1, t2, op).
|
I tested manually the fix using https://bugs.python.org/issue35961#msg335333 patch. Without this change, "./python -m test -F -j0 test_slice test_slice test_slice test_slice test_slice test_slice test_slice test_slice" fails in less than 50 runs. With this change, I had to interrupt the command after 1500 runs (no crash). |
pablogsal
left a comment
pablogsal
left a comment
There was a problem hiding this comment.
LGTM
Only one small comment
| if (t1 == NULL) { | ||
| return NULL; | ||
| } | ||
| PyObject_GC_UnTrack(t1); |
There was a problem hiding this comment.
Does it makes sense to add a small comment here explaining why we untrack the tuples?
|
Also, isn't it |
Yup, |
tim-one
left a comment
tim-one
left a comment
There was a problem hiding this comment.
Good catch! I agree with the earlier comments, especially that this is obscure enough to deserve a comment about why the temp tuples are being exempted from gc.
|
I wrote PR #11830: the other possible fix. |
|
I'm going to merge PR #11830 instead which is safer. |
5 participants
Fix a crash in slice_richcompare(): ensure that the 2 temporary
internal tuples used for the comparison are not tracked by the
garbage collector, since they use borrowed references.
The crash (or assertion error) occurred if a garbage collection
occurred during slice_richcompare(), especially while calling
PyObject_RichCompare(t1, t2, op).
https://bugs.python.org/issue35961