Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix memory leak in logging.enable #152

Merged
merged 3 commits into from Mar 18, 2019
Merged

Fix memory leak in logging.enable #152

merged 3 commits into from Mar 18, 2019

Conversation

c-w
Copy link
Contributor

@c-w c-w commented Mar 1, 2019

The logging.enable function caches references to the LoggingHandler instances that it creates in a dictionary private to the module.

This means that references to the handler objects survive even if they were removed from all loggers that referenced them, causing two undesirable side-effects:

  1. The LoggingHandler instances created by logging.enable can never be reclaimed by the garbage collector.

  2. Removing all references to the LoggingHandler from the logger doesn't mark the instrumentation key as "no longer used" in the logging.enable module.

This change fixes these issues by replacing the cache with a WeakValueDictionary: as soon as the last reference to the LoggingHandler is removed (e.g. when some_logger.removeHandler(...) is called), the WeakValueDictionary will also evict its cached entry.

c-w and others added 3 commits March 1, 2019 14:08
The `logging.enable` function caches references to the LoggingHandler
instances that it creates in a dictionary private to the module.

This means that references to the handler objects survive even if they
were removed from all loggers that referenced them, causing two
undesirable side-effects:

1) The LoggingHandler instances created by `logging.enable` can never be
   reclaimed by the garbage collector.

2) Removing all references to the LoggingHandler from the logger doesn't
   mark the instrumentation key as "no longer used".

This change fixes these issues by replacing the cache with a
WeakValueDictionary: as soon as the last reference to the LoggingHandler
is removed (e.g. when `some_logger.removeHandler(...)` is called), the
WeakValueDictionary will also evict its cached entry.
@SergeyKanzhelev SergeyKanzhelev merged commit d7dcad1 into microsoft:develop Mar 18, 2019
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.

None yet

2 participants