gh-87298: Add tests for find_in_strong_cache() bug in _zoneinfo #24829

ZackerySpytz · 2021-03-11T22:56:33Z

Co-Authored-By: Paul Ganssle [email protected]

https://bugs.python.org/issue43132

Issue: Incorrect handling of PyObject_RichCompareBool() in the _zoneinfo module #87298

Co-Authored-By: Paul Ganssle <[email protected]>

github-actions · 2021-04-12T00:11:22Z

This PR is stale because it has been open for 30 days with no activity.

python-cla-bot · 2025-04-18T09:49:12Z

All commit authors signed the Contributor License Agreement.

StanFromIreland · 2025-06-29T16:51:27Z

Serhiy, since you merged #24450, maybe you can review the tests?

miss-islington-app · 2025-07-01T14:55:27Z

Thanks @ZackerySpytz for the PR, and @serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13, 3.14.
🐍🍒⛏🤖

…pythonGH-24829) (cherry picked from commit 12ce16b) Co-authored-by: Zackery Spytz <[email protected]> Co-authored-by: Paul Ganssle <[email protected]> Co-authored-by: Serhiy Storchaka <[email protected]>

bedevere-app · 2025-07-01T14:55:37Z

GH-136181 is a backport of this pull request to the 3.14 branch.

…pythonGH-24829) (cherry picked from commit 12ce16b) Co-authored-by: Zackery Spytz <[email protected]> Co-authored-by: Paul Ganssle <[email protected]> Co-authored-by: Serhiy Storchaka <[email protected]>

bedevere-app · 2025-07-01T14:55:41Z

GH-136182 is a backport of this pull request to the 3.13 branch.

pganssle · 2025-07-01T14:41:55Z

Lib/test/test_zoneinfo/test_zoneinfo.py

@@ -1507,6 +1530,26 @@ def test_clear_cache_two_keys(self):
        self.assertIsNot(dub0, dub1)
        self.assertIs(tok0, tok1)

+    def test_clear_cache_refleak(self):


Apparently I wrote this function 4 years ago, but coming back to it now it is opaque as heck, so we should probably describe what this test is about.

Suggested change

def test_clear_cache_refleak(self):

def test_clear_cache_refleak(self):

# bpo-43132: The portion of the C implementation of ZoneInfo that

# ejects specific keys from the strong cache (i.e. the LRU cache) was

# leaking references if comparing to the key object failed; because the

# strong cache is not currently implemented for subclasses, this test

# only really applies to the C implementation of the ZoneInfo class.

pganssle · 2025-07-01T15:06:33Z

Lib/test/test_zoneinfo/test_zoneinfo.py

+        self.klass(key)
+        key.allow_comparisons = False
+        try:
+            # Note: This is try/except rather than assertRaises because


I realize that I wrote this function, but it kind of bothers me that this can silently become a no-op (and in fact this is silently a no-op for all subclasses ZoneInfo). I think that the only way around it is fairly annoying if we want to allow tests to run in parallel, which is that we would need to acquire a lock around anything that deals with the cache.

pganssle · 2025-07-01T15:07:10Z

Lib/test/test_zoneinfo/test_zoneinfo.py

+            __hash__ = str.__hash__
+
+        key = Stringy("America/Los_Angeles")
+        self.klass(key)


To increase the chance that this actually works:

Suggested change

self.klass(key)

self.klass.clear_cache(only_keys="America/Los_Angeles")

self.klass(key)

pganssle · 2025-07-01T15:07:40Z

Lib/test/test_zoneinfo/test_zoneinfo.py

@@ -1507,6 +1530,26 @@ def test_clear_cache_two_keys(self):
        self.assertIsNot(dub0, dub1)
        self.assertIs(tok0, tok1)

+    def test_clear_cache_refleak(self):


We may also want to skip this test if self.klass is not exactly the C implementation of ZoneInfo, since otherwise it's a no-op.

bedevere-app · 2025-07-01T15:07:51Z

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

pganssle · 2025-07-01T15:08:08Z

Oof, missed the window for this by 12 minutes.

GH-24829) (GH-136182) (cherry picked from commit 12ce16b) Co-authored-by: Zackery Spytz <[email protected]> Co-authored-by: Paul Ganssle <[email protected]> Co-authored-by: Serhiy Storchaka <[email protected]>

GH-24829) (GH-136181) (cherry picked from commit 12ce16b) Co-authored-by: Zackery Spytz <[email protected]> Co-authored-by: Paul Ganssle <[email protected]> Co-authored-by: Serhiy Storchaka <[email protected]>

serhiy-storchaka · 2025-07-01T16:13:59Z

Sorry, the PR was already on auto-merge.

the-knights-who-say-ni added the CLA signed label Mar 11, 2021

bedevere-bot added tests Tests in the Lib/test dir awaiting review labels Mar 11, 2021

bpo-43132: Add tests for find_in_strong_cache() bug in _zoneinfo

1f7a1fa

Co-Authored-By: Paul Ganssle <[email protected]>

ZackerySpytz force-pushed the bpo-43132-find_in_strong_cache-_zoneinfo-tests branch from 7c640db to 1f7a1fa Compare March 12, 2021 01:48

github-actions bot added the stale Stale PR or inactive for long period of time. label Apr 12, 2021

ZackerySpytz mannequin mentioned this pull request Apr 10, 2022

Incorrect handling of PyObject_RichCompareBool() in the _zoneinfo module #87298

Open

ezio-melotti removed the CLA signed label Jul 13, 2022

github-actions bot removed the stale Stale PR or inactive for long period of time. label Aug 5, 2022

Merge branch 'main' into bpo-43132-find_in_strong_cache-_zoneinfo-tests

ae1ba75

iritkatriel added the skip news label May 14, 2023

iritkatriel requested a review from pganssle May 14, 2023 13:49

erlend-aasland changed the title ~~bpo-43132: Add tests for find_in_strong_cache() bug in _zoneinfo~~ gh-87298: Add tests for find_in_strong_cache() bug in _zoneinfo Jan 5, 2024

Merge branch 'main' into bpo-43132-find_in_strong_cache-_zoneinfo-tests

41298bd

erlend-aasland self-requested a review January 5, 2024 13:01

StanFromIreland requested a review from serhiy-storchaka June 29, 2025 16:51

serhiy-storchaka added 2 commits July 1, 2025 16:05

Merge branch 'main' into bpo-43132-find_in_strong_cache-_zoneinfo-tests

7a4accd

Fix and simplify tests.

446d058

serhiy-storchaka enabled auto-merge (squash) July 1, 2025 14:37

serhiy-storchaka added needs backport to 3.13 bugs and security fixes needs backport to 3.14 bugs and security fixes labels Jul 1, 2025

serhiy-storchaka merged commit 12ce16b into python:main Jul 1, 2025
43 checks passed

bedevere-app bot removed the awaiting review label Jul 1, 2025

bedevere-app bot removed the needs backport to 3.14 bugs and security fixes label Jul 1, 2025

bedevere-app bot removed the needs backport to 3.13 bugs and security fixes label Jul 1, 2025

pganssle requested changes Jul 1, 2025

View reviewed changes

bedevere-app bot added the awaiting changes label Jul 1, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

gh-87298: Add tests for find_in_strong_cache() bug in _zoneinfo #24829

gh-87298: Add tests for find_in_strong_cache() bug in _zoneinfo #24829

ZackerySpytz commented Mar 11, 2021 •

edited by bedevere-app bot

Loading

Uh oh!

github-actions bot commented Apr 12, 2021

Uh oh!

python-cla-bot bot commented Apr 18, 2025 •

edited

Loading

Uh oh!

StanFromIreland commented Jun 29, 2025 •

edited

Loading

Uh oh!

Uh oh!

miss-islington-app bot commented Jul 1, 2025

Uh oh!

bedevere-app bot commented Jul 1, 2025

Uh oh!

bedevere-app bot commented Jul 1, 2025

Uh oh!

pganssle Jul 1, 2025

Uh oh!

pganssle Jul 1, 2025

Uh oh!

pganssle Jul 1, 2025

Uh oh!

pganssle Jul 1, 2025

Uh oh!

bedevere-app bot commented Jul 1, 2025

Uh oh!

pganssle commented Jul 1, 2025

Uh oh!

serhiy-storchaka commented Jul 1, 2025

Uh oh!

Uh oh!

-    def test_clear_cache_refleak(self):
+    def test_clear_cache_refleak(self):
+        # bpo-43132: The portion of the C implementation of ZoneInfo that
+        # ejects specific keys from the strong cache (i.e. the LRU cache) was
+        # leaking references if comparing to the key object failed; because the
+        # strong cache is not currently implemented for subclasses, this test
+        # only really applies to the C implementation of the ZoneInfo class.

	self.klass(key)
	self.klass.clear_cache(only_keys="America/Los_Angeles")
	self.klass(key)

Uh oh!

gh-87298: Add tests for find_in_strong_cache() bug in _zoneinfo #24829

gh-87298: Add tests for find_in_strong_cache() bug in _zoneinfo #24829

Conversation

ZackerySpytz commented Mar 11, 2021 • edited by bedevere-app bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Apr 12, 2021

Uh oh!

python-cla-bot bot commented Apr 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

StanFromIreland commented Jun 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

miss-islington-app bot commented Jul 1, 2025

Uh oh!

bedevere-app bot commented Jul 1, 2025

Uh oh!

bedevere-app bot commented Jul 1, 2025

Uh oh!

pganssle Jul 1, 2025

Choose a reason for hiding this comment

Uh oh!

pganssle Jul 1, 2025

Choose a reason for hiding this comment

Uh oh!

pganssle Jul 1, 2025

Choose a reason for hiding this comment

Uh oh!

pganssle Jul 1, 2025

Choose a reason for hiding this comment

Uh oh!

bedevere-app bot commented Jul 1, 2025

Uh oh!

pganssle commented Jul 1, 2025

Uh oh!

serhiy-storchaka commented Jul 1, 2025

Uh oh!

Uh oh!

ZackerySpytz commented Mar 11, 2021 •

edited by bedevere-app bot

Loading

python-cla-bot bot commented Apr 18, 2025 •

edited

Loading

StanFromIreland commented Jun 29, 2025 •

edited

Loading