Skip to content

Let downloader factories be freed once they are no longer used - #8345

Open
aevnsvi wants to merge 1 commit into
pulp:mainfrom
AVSystem:fix-downloader-factory-atexit-leak
Open

aevnsvi wants to merge 1 commit into
pulp:mainfrom
AVSystem:fix-downloader-factory-atexit-leak

Conversation

@aevnsvi

@aevnsvi aevnsvi commented Nov 13, 2026

Copy link
Copy Markdown
Contributor

Closes #8344

Every DownloaderFactory registered its own bound _session_cleanup with atexit, and the atexit registry holds it until the process exits. The content app builds a factory for every file it streams from a remote, so each streamed file kept a factory, its remote and its closed aiohttp session in memory for the life of the worker. Details are in #8344.

What changes

  • The factories are tracked in a module-level weakref.WeakSet, and one atexit hook closes the sessions that are still open at exit. A factory that is no longer used can now be freed.
  • _session_cleanup skips a session its owner has already closed, so the exit hook no longer runs an event loop for it (on Python 3.12+ that failed with "There is no current event loop" for every factory ever created, see ClientSession teardown runs after event loop closes #6442).
  • No API change; the content handler keeps closing the session in its finally block as before.

Testing

  • Two unit tests in test_downloader_factory.py:

    • a factory whose session was closed is freed after gc.collect();
    • the exit hook closes a session that is still open, and a second run does nothing.

    Both fail without the change.

  • Reproduction: 300 factories built and closed the way the content handler does it; all 300 are still alive after gc.collect() on main, none with the change.

  • The pulpcore unit suite gives the same results with and without the change.

Checklist

  • Commits are cleanly separated with meaningful messages (simple features and bug fixes should be squashed to one commit)
  • A changelog entry or entries has been added for any significant changes
  • Follows the Pulp policy on AI Usage
  • (For new features) - User documentation and test coverage has been added

Every DownloaderFactory registered its own bound method with atexit, and
the atexit registry holds it until the process exits. The content app
builds a factory for every file it streams from a remote, so each
streamed file kept a factory, its remote and its closed aiohttp session
in memory for the life of the worker.

Track the factories in a weak set and close the sessions still open at
exit from one atexit hook. Sessions already closed by their owner are
skipped, which also avoids running an event loop for them at exit.

Closes: pulp#8344

This branch has not been deployed

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Content app keeps a DownloaderFactory, its Remote and its HTTP session in memory for every file streamed from a remote

1 participant