Skip to content

gh-161143: Release completed buffered Pool iterator registrations - #161144

Open
JoaoMarcos44 wants to merge 1 commit into
python:mainfrom
JoaoMarcos44:gh-161143-release-pool-semaphores
Open

JoaoMarcos44 wants to merge 1 commit into
python:mainfrom
JoaoMarcos44:gh-161143-release-pool-semaphores

Conversation

@JoaoMarcos44

@JoaoMarcos44 JoaoMarcos44 commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor

Closes #161143.

Fully consumed buffered Pool.imap() and imap_unordered() iterations
currently leave semaphore registrations behind in a live pool. This change
removes each registration at completion before clearing the iterator's
reference to the pool.

Root cause

A buffered iterator creates a threading.Semaphore and adds it to
Pool._taskqueue_buffersize_semaphores. This registry has a legitimate
shutdown purpose: close() and termination use it to wake task submission
that may be blocked by the buffer limit.

The lifetime of that registration is longer than intended:

  1. IMapIterator.__init__() registers the semaphore and retains the pool in
    self._pool.
  2. Once all results have arrived, IMapIterator._set() or
    IMapUnorderedIterator._set() removes the iterator from the result cache
    and sets self._pool = None. The equivalent completion branch in
    _set_length() handles cases where the length becomes known after the
    results, including empty input.
  3. When the caller exhausts the iterator, _stop_iterator() tries to discard
    its semaphore from the pool's registry, but only if self._pool is not None.
    That reference has already been cleared, so the discard is skipped.
  4. The iterator can be discarded, but the live pool's set still owns a strong
    reference to the semaphore. Garbage collection therefore cannot reclaim
    that object merely because the caller has finished consuming the results.

Removing the iterator from the result cache does not remove this separate
registration. The original implementation is in
Lib/multiprocessing/pool.py at the audited base.

Consequences and observed impact

  • A reused pool accumulates one stale semaphore registration per completed
    buffered iterator, rather than per individual result. This can happen even
    when the input is empty.
  • Fully consuming the results does not avoid the retention. A service or batch
    worker that repeatedly maps small inputs through one long-lived pool can
    keep accumulating these Python objects after the real work has finished.
  • The registry also contains stale entries when shutdown traverses it.
    Additional traversal work follows from the code path; its latency impact
    has not been benchmarked.
  • The 10-iteration reproduction below leaves 10 registrations in the
    unmodified module and 0 with the patch. In a separate unpatched
    2,000-iteration experiment, 2,000 semaphores remained and tracemalloc
    measured 2,693,553 additional bytes of Python allocations
    (approximately 2.7 MB). This is a local measurement, not process RSS or a
    production growth-rate forecast.

The evidence concerns retained Python semaphore objects. It does not show
leaked worker processes, incorrect returned results, an out-of-memory failure,
or a universal shutdown hang. No official P0/P1 severity or security impact is
claimed.

Minimal reproduction

ThreadPool shares the affected iterator implementation with the process
pool and keeps this example small:

from multiprocessing.pool import ThreadPool

with ThreadPool(1) as pool:
    for _ in range(10):
        assert list(pool.imap(str, [1], buffersize=1)) == ["1"]
    print(len(pool._taskqueue_buffersize_semaphores))

Observed before the fix: 10. Expected after these completed iterations:
0. The private registry is inspected only to expose the retained
references.

The same behavior was reproduced with a spawn process pool,
imap_unordered(), and chunksizes 1 and 2. A control without buffersize
retained no registrations. The API being tested is from the 3.16 source;
running this snippet on an older, unmodified module that lacks buffersize
will not reproduce this defect.

Correction and why it addresses the cause

The fix removes the semaphore from the pool's registry before clearing
self._pool. A shared _release_pool() helper is called by each completion
path and by _stop_iterator(); using discard() makes repeated removal
harmless.

Completion means that the task iterable's length is known and all its results
have arrived. At that point, that iterator's task generator no longer needs
the registry entry to be woken during shutdown. This does not cancel pending
work, consume the caller's results, or destroy the pool. The iterator still
owns its own semaphore reference while it needs it.

The completion and exhaustion paths use the iterator's existing condition
lock. Since the result handler can now remove registry entries, termination
uses a list(set) snapshot, matching the existing close() implementation.
The snapshot rationale was reviewed against the CPython implementation and a
controlled iterator interleaving; it was not validated on a free-threaded
runtime.

Changes and regression coverage

  • Lib/multiprocessing/pool.py: release registrations from ordered,
    unordered and length-setting completion paths, and take a list snapshot
    when termination processes the registry.
  • Lib/test/_test_multiprocessing.py: add
    test_imap_with_buffersize_releases_semaphores, covering ordered and
    unordered iteration, chunksizes 1 and 2, and empty/nonempty inputs.
    It checks both returned results and the absence of stale registrations.
    Manager pools are skipped because the check requires the real pool's
    internal registry.
  • Misc/NEWS.d/next/Library/: add a Library entry describing the memory-leak
    fix.

The test checks the registry after iterator exhaustion and the existing
condition-lock synchronization, without relying on an arbitrary sleep.
The patch does not change the public method signatures or buffer-size policy.

Code and test commit.
git diff --check passed for the submitted changes.

Validation environment and limits

Local validation used Windows CPython 3.15.0b2 loading the audited 3.16
multiprocessing.pool source
, based on
4b21d154b4864531641f90e1b87108575e673d75.
It was not a native CPython 3.16 build or its native regrtest runner.
No free-threaded runtime was executed locally.

The new regression method produced 16 failing subtests across the thread and
process runs before the fix and none afterward. A selection of 64 test cases
and 68 additional bounded scenarios passed with the patched module. The
selected-method harness extracted source test methods and supplied their
parameters for the 3.15 host; these counts must not be read as a full native
3.16 test-suite run. The additional scenarios cover worker/iterable errors,
out-of-order results, and concurrent completion/shutdown.

Detailed validation and suggested native commands
provide the scope and limitations of these checks.

Victorian infographic

The ledger represents the pool's registry and the tokens represent Python
semaphore objects. The illustration shows why completed work must release
its registry entry; it does not depict leaked OS handles or imply that active
work is cancelled.

Victorian engineering diagram: a buffered iterator registers a permit, finished operations leave entries behind, and the fix removes the registry entry at completion.

AI tools assisted the implementation, test harnesses, review and illustration.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 1, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-01T22:53:57.807030Z d7e7cb4 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@gpshead gpshead added the AI-needs-explanation AI-assisted work that needs further explanation and follow up from the contributor label Nov 1, 2026
@bedevere-app

bedevere-app Bot commented Nov 1, 2026

Copy link
Copy Markdown

Thank you for your interest in contributing to Python. This is a community-driven project, and we want it to be a welcoming place to learn and collaborate.

Contributors are responsible for reviewing and understanding what they submit, checking its claims, and being able to explain changes in their own words. This helps reviewers focus on the contribution and leaves more time to support other contributors.

This contribution appears to have been authored with AI assistance. Please explain your contribution in your own words and describe how you checked its correctness.

Please read our guidelines for using AI tools.

Copy link
Copy Markdown
Contributor Author

In response to the request for an explanation, here is how this was found and checked.

How the bug was found

The investigation started with an AI-assisted source-code review of the lifetime of buffered Pool.imap() and imap_unordered() iterators. The relevant inconsistency is in cleanup order: completion removes the iterator from the result cache and clears self._pool, but the later semaphore deregistration in _stop_iterator() requires that pool reference to still exist.

The pool's semaphore registry is separate from the result cache. Clearing the iterator therefore leaves the semaphore reachable through the live pool, even after the caller has consumed all results.

How the behavior was checked

A bounded reproduction repeatedly completed ten buffered mappings through the same one-worker ThreadPool. All returned values were correct, but the semaphore registry contained ten entries afterward. With the patch it contained zero. The control without buffersize also retained zero registrations. The retention was additionally reproduced with a spawn process pool, ordered/unordered iteration, and chunksizes 1 and 2.

The added regression test, test_imap_with_buffersize_releases_semaphores, checks both the results and the registry after exhaustion, including empty inputs. Its thread/process executions produced 16 failing subtests before the change and none afterward. Further selected tests exercised exceptions, out-of-order results, partial consumption, and concurrent completion/shutdown.

Why this change addresses it

The patch removes the registry entry when completion is established, before clearing the pool reference. At that point the iterable's length is known and all results have arrived, so that iterator's task generator no longer needs a registry entry to be woken during shutdown. This does not cancel active work or discard buffered results. Termination also uses a list snapshot of the registry, matching close(), because completion can now remove entries concurrently.

Human validation and AI assistance

The submitter has confirmed that the contribution received human validation. That confirmation is separate from the automated test executions described above; it is not being presented as a claim that a human manually reran those tests.

AI assisted the investigation, implementation, test harnesses, and preparation of this explanation. The reported local runs used Windows CPython 3.15.0b2 loading the audited 3.16 module, rather than a native 3.16 build. No free-threaded runtime was tested locally. Those limits are recorded in the PR and validation notes.

JoaoMarcos44 commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor Author

When convenient, @gpshead, could you please review the explanation above and take a look at the PR? Please let me know if any further information is needed to address AI-needs-explanation. Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI-needs-explanation AI-assisted work that needs further explanation and follow up from the contributor awaiting review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

multiprocessing.Pool.imap with buffersize retains semaphore registrations after completion

2 participants