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

bpo-41568: Fix refleaks in zoneinfo subclasses #21907

Merged
merged 2 commits into from
Aug 17, 2020

Conversation

pganssle
Copy link
Member

@pganssle pganssle commented Aug 17, 2020

There are two refleaks related to subclassing the C implementation of ZoneInfo:

  1. __init_subclass__ leaks a reference to the subclass's weak cache.
  2. ZoneinfoSubclass.clear_cache() incidentally sets zoneinfo.ZoneInfo's pointer to its strong cache to NULL, thus leaking the old strong cache (and forcing ZoneInfo to create a new one).

This fixes both of these issues.

https://bugs.python.org/issue41568

This was leaking a reference to the weak cache dictionary for every
ZoneInfo subclass created.
The previous version of the code accidentally cleared the global
ZONEINFO_STRONG_CACHE variable (and inducing `ZoneInfo` to create a new
strong cache) on calls to a subclass's `clear_cache()`. This would not
affect guaranteed behavior, but it's still not the right thing to do
(and it caused reference leaks).
Copy link
Member

@pablogsal pablogsal left a comment

Choose a reason for hiding this comment

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

I can confirm that this works:

❯ ./python -m test test_zoneinfo -R :
0:00:00 load avg: 7.77 Run tests sequentially
0:00:00 load avg: 7.77 [1/1] test_zoneinfo
beginning 9 repetitions
123456789
.........

== Tests result: SUCCESS ==

1 test OK.

Total duration: 10.0 sec
Tests result: SUCCESS

and the fix LGTM

@pablogsal pablogsal merged commit c3dd7e4 into python:master Aug 17, 2020
@miss-islington
Copy link
Contributor

Thanks @pganssle for the PR, and @pablogsal for merging it 🌮🎉.. I'm working now to backport this PR to: 3.9.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Aug 17, 2020
* Fix refleak in C module __init_subclass__

This was leaking a reference to the weak cache dictionary for every
ZoneInfo subclass created.

* Fix refleak in ZoneInfo subclass's clear_cache

The previous version of the code accidentally cleared the global
ZONEINFO_STRONG_CACHE variable (and inducing `ZoneInfo` to create a new
strong cache) on calls to a subclass's `clear_cache()`. This would not
affect guaranteed behavior, but it's still not the right thing to do
(and it caused reference leaks).
(cherry picked from commit c3dd7e4)

Co-authored-by: Paul Ganssle <[email protected]>
@bedevere-bot bedevere-bot removed the needs backport to 3.9 only security fixes label Aug 17, 2020
@bedevere-bot
Copy link

GH-21912 is a backport of this pull request to the 3.9 branch.

miss-islington added a commit that referenced this pull request Aug 17, 2020
* Fix refleak in C module __init_subclass__

This was leaking a reference to the weak cache dictionary for every
ZoneInfo subclass created.

* Fix refleak in ZoneInfo subclass's clear_cache

The previous version of the code accidentally cleared the global
ZONEINFO_STRONG_CACHE variable (and inducing `ZoneInfo` to create a new
strong cache) on calls to a subclass's `clear_cache()`. This would not
affect guaranteed behavior, but it's still not the right thing to do
(and it caused reference leaks).
(cherry picked from commit c3dd7e4)

Co-authored-by: Paul Ganssle <[email protected]>
shihai1991 pushed a commit to shihai1991/cpython that referenced this pull request Aug 20, 2020
* Fix refleak in C module __init_subclass__

This was leaking a reference to the weak cache dictionary for every
ZoneInfo subclass created.

* Fix refleak in ZoneInfo subclass's clear_cache

The previous version of the code accidentally cleared the global
ZONEINFO_STRONG_CACHE variable (and inducing `ZoneInfo` to create a new
strong cache) on calls to a subclass's `clear_cache()`. This would not
affect guaranteed behavior, but it's still not the right thing to do
(and it caused reference leaks).
xzy3 pushed a commit to xzy3/cpython that referenced this pull request Oct 18, 2020
* Fix refleak in C module __init_subclass__

This was leaking a reference to the weak cache dictionary for every
ZoneInfo subclass created.

* Fix refleak in ZoneInfo subclass's clear_cache

The previous version of the code accidentally cleared the global
ZONEINFO_STRONG_CACHE variable (and inducing `ZoneInfo` to create a new
strong cache) on calls to a subclass's `clear_cache()`. This would not
affect guaranteed behavior, but it's still not the right thing to do
(and it caused reference leaks).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants