NotificationsYou must be signed in to change notification settings
Fork34k
Star71.3k

gh-123471: Make concurrent iteration over itertools.permutations and itertools.combinations_with_replacement thread-safe#144402

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Open

eendebakpt wants to merge8 commits intopython:main

base:main

Choose a base branch

fromeendebakpt:cwr_ft

Open

gh-123471: Make concurrent iteration over itertools.permutations and itertools.combinations_with_replacement thread-safe#144402

eendebakpt wants to merge8 commits intopython:mainfromeendebakpt:cwr_ft

+59 −73

Conversation

Copy link

Contributor

eendebakpt commentedFeb 2, 2026•
edited by bedevere-appbot
Loading

We use a critical section similar to the approach in#132814.

Issue:Make concurrent iteration over pairwise, combinations, permutations, cwr, product, etc. from itertools safe under free-threading #123471

eendebakpt added3 commits

February 2, 2026 18:35

Make itertools.combinations_with_replacement thread-safe

56b615e

Make itertools.combinations_with_replacement thread-safe

8139046

Make permutations thread-safe

6370bc4

eendebakpt requested a review fromrhettinger as acode owner

February 2, 2026 17:56

bedevere-appbot added the awaiting review label

Feb 2, 2026

bedevere-appbot mentioned this pull request

Feb 2, 2026

Make concurrent iteration over pairwise, combinations, permutations, cwr, product, etc. from itertools safe under free-threading#123471

Open

remove redundant variable

af909ce

colesbury reviewed

Feb 2, 2026

View reviewed changes

Lib/test/test_free_threading/test_itertools.py Outdated

Comment on lines 92 to 117

		@threading_helper.reap_threads
		def test_combinations_with_replacement(self):
		number_of_threads = 6
		number_of_iterations = 36
		data = tuple(range(2))

		barrier = Barrier(number_of_threads)
		def work(it):
		barrier.wait()
		while True:
		try:
		next(it)
		except StopIteration:
		break

		for _ in range(number_of_iterations):
		cwr_iterator = combinations_with_replacement(data, 2)
		worker_threads = []
		for _ in range(number_of_threads):
		worker_threads.append(
		Thread(target=work, args=[cwr_iterator]))

		with threading_helper.start_threads(worker_threads):
		pass

		barrier.reset()

Copy link

Contributor

Choose a reason for hiding this comment

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

I think this can be simplified and easier to read as something like:

deftest_combinations_with_replacement(self):defwork(it):whileTrue:try:next(it)exceptStopIteration:breakthreading_helper.run_concurrently(work,nthreads=6,args=[combinations_with_replacement(tuple(range(2)),2)])

I think it's generally not worth the CI time to run multiple iterations in the hopes of making bugs more likely. TSan is pretty good at catching these things even if it doesn't crash. If I remove the critical sections, most runs report a data race with TSan.
All or nearly all the tests have the samedef work(). We can extract that too
I don't think we need the@threading_helper.reap_threads.threading_helper.run_concurrently handles that.

Copy link

ContributorAuthor

Choose a reason for hiding this comment

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

The diff is now larger, but the code is indeed much simpler. I kept the@threading_helper.reap_threads for now, claude things we still need it in case of exceptions (I will double check later).

For all the tests I reduced the number of iterations. We could remove the iterations altogether, but I kept it to the test the concurrent exhaustion of the iterator (which is different from the concurrent iteration part). If needed I can remove the multiple iterations altogether.