gh-117657: Fix itertools.count thread safety
Thread safety in count_next. count_next has two modes. slow mode (obj->cnt set to PY_SSIZE_T_MAX), which now uses the object mutex (only if GIL is disabled) and fast mode, which is either simple cnt++ if GIL is enabled, or uses atomic_compare_exchange if GIL is disabled.
- Issue: gh-117657
Can you arrange the #ifdef logic so that the traditional GIL build has EXACTLY the same code that it does now. That shouldn't be difficult since count_next() is only three lines of code.
In general, we don't want to damage the traditional build if we don't have to.
Also, we would like maintainers to be able to easily look at the GIL path to understand what the function does and how it works. After the PR, I personally find it challenging to just read this code that used to be simple and obvious.
Can you arrange the #ifdef logic so that the traditional GIL build has EXACTLY the same code that it does now. That shouldn't be difficult since count_next() is only three lines of code.
Just to be clear, are you suggesting to bring back the original code under an ifdef, like so?
#ifdnef Py_GIL_DISABLED
// the original 3 lines, unmodified
#else
// free-threading code only
#endif
?
Just to be clear, are you suggesting to bring back the original code under an ifdef, like so?
+1 That would be a simple and reasonable way to preserve readability, maintainability, and performance. It would also make clear exactly what the no-gil code should be doing.
Thank you @colesbury and @rhettinger for the constructive feedback. I pushed a revised commit. Please share your comments.
The path for the traditional build seems fine to me. The no-gil path is much more readable than before but I'm not expert enough to sign-off on that path.
Oops, forgot to backport this to 3.13
Thanks @wiggin15 for the PR, and @DinoV for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13. 🐍🍒⛏🤖
Sorry, @wiggin15 and @DinoV, I could not cleanly backport this to 3.13 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 87939bd5790accea77c5a81093f16f28d3f0b429 3.13
GH-120007 is a backport of this pull request to the 3.13 branch.