GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-124397: Add threading.iter_locked by eendebakpt · Pull Request #133908 · python/cpython

Conversation

@eendebakpt

@eendebakpt eendebakpt commented May 11, 2025 •
edited by github-actions Bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

Continuation of #133272
Name was discussed in https://discuss.python.org/t/name-for-new-itertools-object-to-make-iterators-thread-safe/90394

  • Issue: Strategy for Iterators in Free Threading #124397

📚 Documentation preview 📚: https://cpython-previews--133908.org.readthedocs.build/

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
eendebakpt added 3 commits May 10, 2025 23:23
@eendebakpt
Add iter_locked
6313b3d
@eendebakpt
cleanup
5ced0a9
@eendebakpt
cleanup
110332e
@bedevere-app bedevere-app Bot mentioned this pull request May 11, 2025
Strategy for Iterators in Free Threading #124397
Open
@bedevere-app bedevere-app Bot added the awaiting review label May 11, 2025
@eendebakpt
eendebakpt marked this pull request as draft May 11, 2025 19:47
@bedevere-app bedevere-app Bot removed the awaiting review label May 11, 2025
@blurb-it
📜🤖 Added by blurb_it.
b675631
@eendebakpt eendebakpt mentioned this pull request May 11, 2025
gh-124397: Add itertools.serialize (name tbd) #133272
Closed
3 tasks
@eendebakpt
update docs
4406050
@ZeroIntensity

ZeroIntensity commented May 12, 2025

Copy link
Copy Markdown
Member

@serhiy-storchaka, what was the rationale behind putting this in the threading module? I'm not against it, but I'm more in favor of itertools because threading doesn't expose any other utilities like this. Maybe we should add a threading.util module?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
ZeroIntensity
ZeroIntensity reviewed May 12, 2025
View reviewed changes
Comment thread Doc/library/threading.rst

The ``iter_locked`` makes non-atomic iterators atomic::

class non_atomic_iterator:

@ZeroIntensity ZeroIntensity May 12, 2025

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I'm not sure non_atomic_iterator is a good example of something that exists in the wild, because it's not thread-safe on GIL-ful builds either. The thread can arbitrarily switch during execution of Python code, so this would need a lock anyway if concurrent iteration were a use case. For free-threading, __next__ itself should have its own locking to prevent races. How was this design reached?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@eendebakpt eendebakpt Jun 1, 2025

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

The iter_locked allows the non_atomic_iterator to be used in a threaded-setting, whether in the normal or FT build. Do you have suggestions for other tests?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@serhiy-storchaka

serhiy-storchaka commented May 12, 2025

Copy link
Copy Markdown
Member
  1. It depends on the threading lock.
  2. There will be a different implementation in the asyncio module that uses an asyncio lock.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@ZeroIntensity

ZeroIntensity commented May 12, 2025

Copy link
Copy Markdown
Member
  1. It seems reasonable to add a lock_type parameter for different types of locking, e.g. Lock vs RLock.
  2. Why can't there be itertools.serialize and then asyncio.async_serialize? asyncio reimplements plenty of things, so it wouldn't be particularly inconsistent/confusing.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
serhiy-storchaka
serhiy-storchaka reviewed Jun 1, 2025
View reviewed changes
Comment thread Lib/test/test_free_threading/test_threading_iter_locked.py

def __next__(self):
a = next(self.it)
b = next(self.it)

@serhiy-storchaka serhiy-storchaka May 12, 2025

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Add time.sleep(0) (or with small non-zero argument) or Event.wait() with a timeout to force or significantly increase the chance of concurrent access.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
eendebakpt reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
Comment thread Modules/_threadmodule.c Outdated
iter_locked_object *lz = iter_locked_object_CAST(op);
PyObject *result = NULL;

Py_BEGIN_CRITICAL_SECTION(op); // lock on op or lz->it?

@serhiy-storchaka serhiy-storchaka May 12, 2025

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Isn't it a no-op in the GIL build?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@eendebakpt eendebakpt Jun 1, 2025

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Good catch, it is indeed a no-op. The Py_BEGIN_CRITICAL_SECTION is defined using _PyCriticalSection_BeginMutex, but that is not available in the normal build. So we should either add it to the normal build, or use another lock implementation.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
ZeroIntensity
ZeroIntensity reviewed Jun 1, 2025
View reviewed changes
Comment thread Modules/_threadmodule.c Outdated

Py_BEGIN_CRITICAL_SECTION(op); // lock on op or lz->it?
PyObject *it = lz->it;
result = PyIter_Next(it);

@ZeroIntensity ZeroIntensity Jun 1, 2025

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Let's use PyIter_NextItem, PyIter_Next has a weird return value.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
eendebakpt added 6 commits June 1, 2025 21:32
@eendebakpt
review comments
d30ace8
@eendebakpt
use explicit lock
baec5bc
@eendebakpt
Merge branch 'main' into iter_locked
e6ea285
@eendebakpt
fix import
7315c49
@eendebakpt
cleanup after merge to main
b7c9d77
@eendebakpt
reduce duration of test
bb98c52
@eendebakpt
eendebakpt marked this pull request as ready for review June 18, 2025 19:32
@bedevere-app bedevere-app Bot added the awaiting review label Jun 18, 2025
@eendebakpt
eendebakpt requested a review from serhiy-storchaka July 1, 2025 11:25
serhiy-storchaka
serhiy-storchaka reviewed Jul 2, 2025
View reviewed changes

@serhiy-storchaka serhiy-storchaka left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Why a simple Python implementation is not good enough?

What will happen if self-lock an iterator? Is there a way to forcefully unlock it ot prevent self-locking by raising an exception?

def g():
    yield next(it)

it = iter_locked(g())
next(it)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@eendebakpt
use recursive lock
946ee51
@eendebakpt

eendebakpt commented Jul 9, 2025

Copy link
Copy Markdown
Contributor Author

Why a simple Python implementation is not good enough?

Originally I wrote the iter_locked to be in itertools which only has a C module, hence the C implementation. But the only reason for a C implementation would be performance, which might not be very important here. I am happy to switch to a python version.

What will happen if self-lock an iterator? Is there a way to forcefully unlock it ot prevent self-locking by raising an exception?

There was a deadlock, with no way to get out. I changed to a recursive lock and added 2 tests. The behaviour is now similar to the pairwise iterator.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@eendebakpt
Merge branch 'main' into iter_locked
7e3653a
@eendebakpt eendebakpt mentioned this pull request Aug 30, 2025
gh-124397: Add pure Python implementation of threading.iter_locked #138261
Closed
@github-actions

github-actions Bot commented Apr 24, 2026

Copy link
Copy Markdown

This PR is stale because it has been open for 30 days with no activity.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@github-actions github-actions Bot added the stale Stale PR or inactive for long period of time. label Apr 24, 2026
@eendebakpt

eendebakpt commented Oct 23, 2026

Copy link
Copy Markdown
Contributor Author

Has been superseded
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@eendebakpt eendebakpt closed this Oct 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@serhiy-storchaka serhiy-storchaka serhiy-storchaka left review comments
@ZeroIntensity ZeroIntensity ZeroIntensity left review comments

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

awaiting review stale Stale PR or inactive for long period of time.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@eendebakpt @ZeroIntensity @serhiy-storchaka
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.