Skip to content

gh-123471: Make concurrent iteration over itertools.islice thread-safe#144528

Open
eendebakpt wants to merge 8 commits intopython:mainfrom
eendebakpt:islice_ft
Open

gh-123471: Make concurrent iteration over itertools.islice thread-safe#144528
eendebakpt wants to merge 8 commits intopython:mainfrom
eendebakpt:islice_ft

Conversation

@eendebakpt
Copy link
Copy Markdown
Contributor

@eendebakpt eendebakpt commented Feb 5, 2026

We make itertools.islice safe under free-threading (e.g. guard against crashes) by:

  • Atomically getting and setting the mutable fields cnt and next
  • Using the step field as a sentinel for iterator exhaustion
  • Not clearing the underlying iterator upon exhaustion

Note: several other iterators in the itertools module have been made safe using a critical section (e.g. #144402). That approach works here as well and would make the approach a bit more uniform and more readable. Downside of a critical section it that is scales less well.

PyObject *item;
PyObject *it = lz->it;
Py_ssize_t stop = lz->stop;
Py_ssize_t oldnext;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I suggest to just use a critical section, performance of concurrent iteration is not important.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The code owner @rhettinger has been hesitant with changes that effect performance. I have not measured it yet but both this PR or the critical section will probably be performance neutral on the normal build. For the FT build critical sections will probably be a bit slower (when running a single thread, I agree that performance of concurrent iteration does not matter).

I opened a second PR #148348 with the critical section approach (which is a simple change and similar to how most of the other iterations in this module have been handled). Looking at it now, I think I slightly prefer the critical section approach.

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.

2 participants