Skip to content

gh-142414: Unify UNBOUND in Lib/concurrent/interpreters/_queues.py and Lib/concurrent/interpreters/_crossinterp.py. - #142515

Open
note35 wants to merge 9 commits into
python:mainfrom
note35:gh-142414
Open

gh-142414: Unify UNBOUND in Lib/concurrent/interpreters/_queues.py and Lib/concurrent/interpreters/_crossinterp.py.#142515
note35 wants to merge 9 commits into
python:mainfrom
note35:gh-142414

Conversation

@note35

@note35note35 commented Dec 10, 2025

Copy link
Copy Markdown
Contributor

The fix is aligned with bug reporter's observation.

Test with

  1. ./python -m test test_interpreters test_struct (This one triggers OP's issue.)
  2. Repro script by Victor.
  3. ./python -m test test_interpreters test_concurrent_futures.test_interpreter_pool (This one triggers the forever loop after the initial fix.)

Comment threadLib/concurrent/interpreters/_queues.py Outdated

@ZeroIntensityZeroIntensity left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This needs a news entry, please add one. You can use blurb-it or the blurb command-line tool.

Comment threadLib/concurrent/interpreters/_queues.py Outdated
@note35note35 changed the title gh-142414: Add register_unbound in _crossinterp.py and initialize it in _queues.pygh-142414: Align UNBOUND in Lib/concurrent/interpreters/_queues.py and Lib/concurrent/interpreters/_crossinterp.py.Dec 10, 2025
@note35note35 changed the title gh-142414: Align UNBOUND in Lib/concurrent/interpreters/_queues.py and Lib/concurrent/interpreters/_crossinterp.py.gh-142414: Unify UNBOUND in Lib/concurrent/interpreters/_queues.py and Lib/concurrent/interpreters/_crossinterp.py.Dec 10, 2025

@ZeroIntensityZeroIntensity left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think it would also be good to add a dedicated test for this, similar to Victor's small repro in the original issue. This bug only occurs under some special circumstances in the current test suite, which isn't ideal.

Comment threadMisc/NEWS.d/next/Library/2025-12-10-15-27-58.gh-issue-142414.zTHgP-.rst Outdated
@note35

Copy link
Copy Markdown
ContributorAuthor

I also created another PR to remove singleton - #142553

Comment threadLib/concurrent/interpreters/_crossinterp.py Outdated
Comment threadMisc/NEWS.d/next/Library/2025-12-10-15-27-58.gh-issue-142414.zTHgP-.rst Outdated
…THgP-.rst
Co-authored-by: Peter Bierma <zintensitydev@gmail.com>
We don't need to make singleton picklabe based on the current tests.
Comment on lines +430 to +431
except queues.QueueEmpty:
pass

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What happened here?

@note35note35Dec 11, 2025

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

./python -m test test_interpreters test_concurrent_futures.test_interpreter_pool Using random seed: 1867170309
0:00:00 load avg: 0.27 Run 2 tests sequentially in a single process
0:00:00 load avg: 0.27 [1/2] test_interpreters
0:00:24 load avg: 0.48 [1/2] test_interpreters passed
0:00:24 load avg: 0.48 [2/2] test_concurrent_futures.test_interpreter_pool
... (running forever without catching queues.QueueEmpty)
deftest_blocking_with_limited_workers(self):
ready=queues.create()
blocker=queues.create()
defrun(taskid, ready, blocker):
ready.put_nowait(taskid)
blocker.get()
numtasks=10futures= []
withself.executor_type(4) asexecutor:
foriinrange(numtasks):
fut=executor.submit(run, i, ready, blocker)
futures.append(fut)
pending=numtaskswhilepending>0:
done=0for_inrange(pending):
try:
ready.get(timeout=1)
# https://github.com/python/cpython/blob/2a820e2b9cf9e470d9f5342019fca3fe7f4ed7bc/Lib/concurrent/interpreters/_queues.py#L261C20-L261C30# test_interpreters test_concurrent_futures.test_interpreter_pool # Before: queue.get() throws interpreters.QueueEmpty# After: queue.get() throws queues.QueueEmpty# test_concurrent_futures.test_interpreter_pool # Before: queue.get() throws interpreters.QueueEmpty.# After: queue.get() throws interpreters.QueueEmptyexceptinterpreters.QueueEmpty:
passexceptqueues.QueueEmpty:
passelse:
# If the error is not caught, the loop is running forever.done+=1pending-=donefor_inrange(done):
blocker.put_nowait(None)

Why does running test_interpreters + test_concurrent_futures.test_interpreter_pool make the queue.get() throws queues.QueueEmpty after having the same UNBOUND object in _queues.py and _crossinterp.py?

FWIK, cpython/Lib/concurrent/interpreters/init.py loads queues.QueueEmpty as interpreters.QueueEmpty, and before this PR, queue.get() is expected to throw interpreters.QueueEmpty,

For some reasons, queue.get() throws _queues.QueueEmpty after running test_interpreters. This may be another thing to explore further (I think it depends on how the sub-interpreter is implemented. Do you have any knowledge about this?

Given this is a test fix, maybe it's okay to have a further dive deep separately? I think this so far can be reproduced only during a combination of test_interpreters + test_concurrent_futures.test_interpreter_pool. (which is likely rare and not reported?) I'd also like to add a TODO gh-XXX, but we don't yet have enough context to make an issue...)

@encukouencukou left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This looks like a good step forward, even if it found another issue.
@ZeroIntensity, are you OK merging this & leaving QueueEmpty for later?

@ZeroIntensity

Copy link
Copy Markdown
Member

The QueueEmpty change makes me very nervous. I can't tell if that's a new problem introduced by this PR, or just an existing issue unveiled.

If you want to merge this to get tests passing, I think that's fine. Long term, something like #142577 or #145184 will work better.

@Allactaga

Allactaga commented Feb 26, 2026

Copy link
Copy Markdown

Hi, @ZeroIntensity, this is definitely an existing issue unveiled. While there are a number of PRs addressing the "singleton issue", the root cause of #142414 is the redefinition of objects when importlib.reload is called. Not only UNBOUND is redefined, but so is QueueEmpty. Identity gets broken and as a result QueueEmpty is not recognised by except clauses where it is expected to.

Regardless what path forward is chosen for the refactoring of UnboundItem, we still need to merge #144096.

@Allactaga

Allactaga commented Feb 26, 2026

Copy link
Copy Markdown

this is definitely an existing issue unveiled

Having said that, I do believe that the following change should not be merged:

exceptqueues.QueueEmpty:
pass

@github-actions

Copy link
Copy Markdown

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

@github-actionsgithub-actionsBot added the stale Stale PR or inactive for long period of time. label May 3, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting mergestaleStale PR or inactive for long period of time.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@note35@ZeroIntensity@Allactaga@encukou