gh-158031: Keep the iter sentinel alive during comparison - #158046

This introduces contention on the FT build. How does it affect performance? PRs touching builtins are delicate and should not just be generated through an LLM. Anyone submitting such PRs should have a deeper understanding of the interpreter and the consequences so please keep in mind that we rather have core devs tackle such issues.

Fair point on the free threaded build. I dont have a benchmark so I cant tell you how the extra reference behaves under contention

I've checked the bug itself. The comparison was using a borrowed sentinel and a reentrant "eq" could free it before the comparison finished. With this extra reference, the tests for that case passed and the object stays alive until the comparison is done

I agree that builtins are delicate and that this is the kind of change you'd rather have a core dev to handle. I'm happy to adjust it, or leave it, whichever you prefer. I may have underestimated that..

I dont have a benchmark so I cant tell you how the extra reference behaves under contention

Please do so then. We usually expect a PR that changes performance critical code to justify the change (the change is correct but the impact is the hard question)

I ran next() on iter(callable, sentinel) on the free-threaded build, macOS 26.5 arm64, with --disable-gil --enable-optimizations --with-lto. The sentinel is a fresh object(), so the refcount actually changes. Just one thread.

main: 26.4 ns +/- 0.3 ns
this PR: 29.1 ns +/- 0.8 ns

Little slower. Around 2.7 ns per "next()". The callable only returns 0, so this is pretty much the worst case. The extra incref/decref is a big chunk of a tiny loop. I didnt try several threads sharing the same sentinel..

Maybe worth looking for another way to fix this?

main-iter.json
pr-iter.json

The other thing we can do is: do nothing. Sometimes it is not worth it just for purity as such cases are rare. It slows down every code path for theoretical issues. OTOH, the sentinel form is also rarer. We can document this as a real crasher.

cc @kumaraditya303

Ok, that makes sense.
Happy to help if there's anything useful I can do