Skip to content

PYTHON-5418 Support subinterpreters and InterpreterPoolExecutor - #3061

Merged
blink1073 merged 25 commits into
mongodb:mainfrom
blink1073:PYTHON-5418
Sep 24, 2026
Merged

blink1073 merged 25 commits into
mongodb:mainfrom
blink1073:PYTHON-5418

Conversation

@blink1073

@blink1073 blink1073 commented Sep 22, 2026 •

Copy link
Copy Markdown
Member

PYTHON-5418

Changes in this PR

  • PeriodicExecutor starts non-daemon threads in subinterpreters, where daemon threads are disallowed. The monitor shutdown handler is registered with threading._register_atexit (only in those interpreters) so executors are stopped and joined at interpreter teardown; main-interpreter shutdown ordering is unchanged.
  • Added tests for PeriodicExecutor in a subinterpreter and a dedicated test_subinterpreters.py covering direct subinterpeter and InterpreterPoolExecutor usage with sync and aync clients.
  • Note: an interpreter may block on teardown until in-flight monitor work completes.
  • Almost all of the changes are new tests.

Test Plan

  • The new subinterpreter tests pass on Python 3.14 and 3.15.
  • With the shutdown changes reverted, test_subinterpreters fails, demonstrating regression coverage.

Checklist

Checklist for Author

  • Did you update the changelog (if necessary)?
  • Is there test coverage?
  • Is any followup work tracked in a JIRA ticket? If so, add link(s). (PYTHON-6114 Adds support for testing mod_wsgi 6.0, which requires full support for subinterpreters).

Checklist for Reviewer

  • Does the title of the PR reference a JIRA Ticket?
  • Do you fully understand the implementation? (Would you be comfortable explaining how this code works to someone else?)
  • Is all relevant documentation (README or docstring) updated?

Copilot AI left a comment

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.

Copilot review overview

🟡 Changes recommended

Unresolved test cleanup and optional-import issues remain.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 2 High severity

Open (2)
What changed in this PR

Adds Python 3.14+ subinterpreter and InterpreterPoolExecutor support with updated monitor shutdown handling.

Changes:

  • Supports non-daemon monitor threads in subinterpreters.
  • Registers interpreter-specific shutdown cleanup.
  • Adds synchronous and asynchronous regression tests.
  • Documents the new behavior.
File Summary Findings
test/​test_subinterpreters.py Synchronous subinterpreter tests. Critical: cleanup may fail to join running execution threads (2 votes).
test/​test_periodic_executor.py Synchronous executor teardown tests. None.
test/​asynchronous/​test_subinterpreters.py Asynchronous subinterpreter tests. Critical: cleanup may fail to join running execution threads (2 votes).
test/​asynchronous/​test_periodic_executor.py Asynchronous executor teardown tests. Moderate: optional concurrent.interpreters import should skip cleanly (1 vote).
pymongo/​synchronous/​monitor.py Synchronous monitor shutdown handling. None.
pymongo/​periodic_executor.py Subinterpreter-compatible executor threads. None.
pymongo/​asynchronous/​monitor.py Asynchronous monitor shutdown handling. None.
doc/​changelog.rst Documents subinterpreter support. None.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread test/asynchronous/test_subinterpreters.py
Comment thread test/test_subinterpreters.py
PeriodicExecutor now falls back to a non-daemon thread when daemon threads are disallowed (subinterpreters). The monitor shutdown handler is also registered with threading._register_atexit so executors are stopped and joined before interpreter teardown.

Adds regression tests for PeriodicExecutor in a real subinterpreter and for concurrent MongoClients across subinterpreters (Python 3.14+).
Runs live MongoClients inside interpreters managed by the standard
InterpreterPoolExecutor, mirroring the existing multi-threaded checks.
The pool's interpreters do not allow daemon threads, so this exercises
the non-daemon monitor thread fallback and its shutdown handling.
The worker interpreters may not have the repo root on sys.path (the
stdlib test package shadows the repo's), so the pickled worker function
failed to unpickle.  Submit the builtin exec with a code string that
inserts the main interpreter's sys.path before importing pymongo.
Runs live AsyncMongoClients inside interpreters managed by
InterpreterPoolExecutor. The async client runs its background tasks on
the interpreter's own event loop rather than in threads, covering the
other shutdown path.
Invert the is_running guard in test_subinterpreters: closing an idle
interpreter runs threading._shutdown, which stops and joins pymongo's
monitor threads, so the successful path must close them. Note in the
changelog that both clients are covered by the subinterpreter tests.
Only register the monitor shutdown handler with
threading._register_atexit when the interpreter disallows daemon
threads, preserving normal main interpreter shutdown ordering.
Test AsyncPeriodicExecutor in a subinterpreter in the async suite,
tighten the code comments, and note that both clients are supported in
the changelog.
Add test/asynchronous/test_subinterpreters.py with the subinterpreter
and InterpreterPoolExecutor client tests, written once in async style
and mirrored by synchro. Remove them from test_threads.py, which is
not mirrored. Collapse the periodic executor test's duplicated worker
blocks into one block and make its async target a coroutine.
Commit 623ecb0 tightened the comment in the async monitor but did
not regenerate the sync mirror.
…preter

The sync worker blocks on an event set by the target's first run, so
the interpreter is destroyed with the monitor live in its interval
loop. The async worker yields to its loop once, which runs the monitor
task's first step.
A failed assertion left the non-daemon worker threads running, which
blocked the test process on exit and leaked the running interpreters.
Join the workers in the cleanup path before closing the interpreters.

Copilot AI left a comment

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.

Copilot review overview

🟡 Changes recommended

New asynchronous tests perform blocking waits on their event loops.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1 Medium severity

Open (1)
Resolved since last review (2)

Comment thread test/asynchronous/test_subinterpreters.py Outdated
@codecov

codecov Bot commented Sep 22, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 73.33333% with 4 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
pymongo/periodic_executor.py 69.23% 4 Missing ⚠️

📢 Thoughts on this report? Let us know!

The queue read in the subinterpreter worker and the pool future waits
blocked the event loop, freezing the client tasks under test. The
async suite waits through asyncio.to_thread instead, while the sync
suite keeps the direct calls.
…y blocks

Test that PeriodicExecutor.open() starts a non-daemon thread when the
daemon assignment raises, and exclude the monitor shutdown registration
blocks, which only run in the sync twin (the async source's block is
dead code).
@blink1073
blink1073 requested a balanced review from Copilot September 22, 2026 15:39
@blink1073
blink1073 marked this pull request as draft September 22, 2026 15:39

Copilot AI left a comment

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.

Copilot review overview

🟡 Changes recommended

The interpreter-pool tests do not execute Future.result(), allowing worker failures to pass silently.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1 Medium severity

Open (1)
Resolved since last review (1)

Comment thread test/asynchronous/test_subinterpreters.py Outdated
The result_stmt interpolation landed outside the worker string as a
set literal, so the pool's futures were never awaited and worker
exceptions were silently ignored. Call future.result() explicitly,
through asyncio.to_thread in the async suite.

Copilot AI left a comment

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.

Copilot review overview

🔵 Needs a closer look

Async tests still perform potentially long blocking operations on the event-loop thread.

Review effort: Balanced
Findings: None

Resolved since last review (1)
Previously missed (3)

In code that hasn't changed since last review

Medium severity Offload blocking queue waits in async tests

test/​asynchronous/​test_subinterpreters.py:128

_get_n() performs queue.get(timeout=30) synchronously, so this async test blocks its event loop while waiting for subinterpreters. Offload the call with await asyncio.to_thread(...) in the async path and preserve the direct call under _IS_SYNC; apply the same fix to the done wait below.

Medium severity Offload thread joins from the async event loop

test/​asynchronous/​test_subinterpreters.py:134

thread.join(60) blocks the async test's event-loop thread. If a subinterpreter is slow or stuck, this prevents the loop from making progress for the whole timeout; await the join through asyncio.to_thread (and apply the same treatment to the cleanup join below).

This issue also appears on line 147 of the same file.

Medium severity Avoid blocking event loop during executor shutdown

test/​asynchronous/​test_subinterpreters.py:196

Although each future.result() is offloaded below, exiting this synchronous context manager calls shutdown(wait=True) on the event-loop thread. Worker-interpreter teardown can wait for in-flight monitor work, so this can still block the async test loop for a long time. Create the executor explicitly and, in the async path, await asyncio.to_thread(executor.shutdown) from finally (while retaining direct shutdown in the generated sync path).

The result_stmt interpolation landed outside the worker string as a
set literal, so the pool's futures were never awaited and worker
exceptions were silently ignored. Call future.result() explicitly,
through asyncio.to_thread in the async suite.
Offload the queue waits, thread joins, and pool shutdown to worker
threads in the async suite, where they would otherwise block the event
loop for up to their full timeouts. The sync suite keeps the direct
calls.

Copilot AI left a comment

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.

Copilot review overview

🔵 Needs a closer look

Interpreter teardown and thread lifecycle changes warrant final human review.

Review effort: Balanced
Findings: None

The Windows proactor loop cannot be created in a subinterpreter:
signal.set_wakeup_fd only works in the main interpreter. Run the async
workers on a selector loop there instead.

Copilot AI left a comment

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.

Copilot review overview

🟡 Changes recommended

Critical thread-start and shutdown-detection flaws block the advertised subinterpreter support, with additional async test issues unresolved.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 2 High severity

Open (2)

Comment thread pymongo/asynchronous/monitor.py Outdated
Comment thread pymongo/periodic_executor.py Outdated

Copilot AI left a comment

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.

Copilot review overview

🔵 Needs a closer look

Interpreter shutdown ordering and thread lifecycle changes require final human review.

Review effort: Balanced
Findings: None

Resolved since last review (2)

@blink1073
blink1073 marked this pull request as ready for review September 23, 2026 10:49
@blink1073
blink1073 merged commit d5934e6 into mongodb:main Sep 24, 2026
92 of 93 checks passed
@blink1073
blink1073 deleted the PYTHON-5418 branch September 24, 2026 19:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants