classification
Title: C implementation of ZoneInfo cannot be subclassed
Type: Stage: resolved
Components: Library (Lib) Versions: Python 3.10, Python 3.9
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: p-ganssle Nosy List: lukasz.langa, miss-islington, p-ganssle, vstinner
Priority: critical Keywords: patch

Created on 2020-06-18 14:26 by p-ganssle, last changed 2020-09-14 17:50 by lukasz.langa. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 20965 merged p-ganssle, 2020-06-18 14:41
PR 21876 merged miss-islington, 2020-08-14 02:38
Messages (11)
msg371817 - (view) Author: Paul Ganssle (p-ganssle) * (Python committer) Date: 2020-06-18 14:26
In the C implementation of zoneinfo.ZoneInfo, __init_subclass__ is not declared as a classmethod, which prevents it from being subclassed. This was not noticed because the tests for ZoneInfo subclasses in C are actually testing zoneinfo.ZoneInfo, not a subclass, due to a mistake in the inheritance tree: https://github.com/python/cpython/blob/8f192d12af82c4dc40730bf59814f6a68f68f950/Lib/test/test_zoneinfo/test_zoneinfo.py#L465-L487


Originally reported on the backport by Sébastien Eustace: https://github.com/pganssle/zoneinfo/issues/82

The fix in the backport is here: https://github.com/pganssle/zoneinfo/pull/83
msg375365 - (view) Author: Paul Ganssle (p-ganssle) * (Python committer) Date: 2020-08-14 02:38
New changeset 87d8287865e5c9f137f6b5cf8c34c2c509eb5e9d by Paul Ganssle in branch 'master':
bpo-41025: Fix subclassing for zoneinfo.ZoneInfo (GH-20965)
https://github.com/python/cpython/commit/87d8287865e5c9f137f6b5cf8c34c2c509eb5e9d
msg375412 - (view) Author: Paul Ganssle (p-ganssle) * (Python committer) Date: 2020-08-14 15:18
New changeset 33d3c64095bcdf9066a3441f6dda5d2e2f4118a8 by Miss Islington (bot) in branch '3.9':
bpo-41025: Fix subclassing for zoneinfo.ZoneInfo (GH-20965) (GH-21876)
https://github.com/python/cpython/commit/33d3c64095bcdf9066a3441f6dda5d2e2f4118a8
msg375418 - (view) Author: Paul Ganssle (p-ganssle) * (Python committer) Date: 2020-08-14 16:50
Łukasz: Would it be possible to backport / cherry-pick the changes from PR GH-21876 (https://github.com/python/cpython/pull/21876) into the 3.9.0 branch? I think this is a fairly severe issue considering that pendulum is planning to use a zoneinfo subclass.

It was merged as commit 33d3c64095bcdf9066a3441f6dda5d2e2f4118a8.

Thanks!
msg375419 - (view) Author: Paul Ganssle (p-ganssle) * (Python committer) Date: 2020-08-14 16:51
Marking as release blocker to put it on the checklist. Feel free to demote it if you decide it should be deferred to 3.9.1.
msg375530 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-08-17 07:47
> New changeset 87d8287865e5c9f137f6b5cf8c34c2c509eb5e9d by Paul Ganssle in branch 'master':
> bpo-41025: Fix subclassing for zoneinfo.ZoneInfo (GH-20965)

This change introduced a reference leak. 3.9 and master branch are affected.

$ make && ./python -m test -R 3:3 test_zoneinfo 
(...)
test_zoneinfo leaked [84, 84, 84] references, sum=252
test_zoneinfo leaked [41, 41, 41] memory blocks, sum=123
(...)

ZoneInfoSubclassTest and CZoneInfoSubclassTest test cases leak.

Example of test method which leaks:

test.test_zoneinfo.test_zoneinfo.CZoneInfoSubclassTest.test_folds_and_gaps
msg375559 - (view) Author: Paul Ganssle (p-ganssle) * (Python committer) Date: 2020-08-17 16:32
There are two refleaks here. One is a reference leaking to the weak cache in `__init_subclass__` (one leak every time a subclass is created), and the other is that when `subclass.clear_cache()` is called, it sets `ZONEINFO_STRONG_CACHE = NULL`, thus causing a reference leak to the parent class's strong cache.

I'll send a PR to fix it shortly.
msg375657 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-08-19 16:14
> test_zoneinfo leaked [84, 84, 84] references, sum=252

Reported as bpo-41568 and fixed there.
msg376712 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-09-11 10:28
Paul: What is the status of this issue? Is it already fixed? It is the release blocker priority.
msg376730 - (view) Author: Paul Ganssle (p-ganssle) * (Python committer) Date: 2020-09-11 13:39
This is fixed in the 3.9 and master branches, it needs to be cherry-picked into the 3.9.0 release (at Łukasz's discretion, of course).

Łukasz: If you cherry-pick GH-20965/GH-21876 into the 3.9.0 release, please also cherry-pick GH-21907/GH-21912, since that fixes the refleak. (PRs are listed as "master"/"backport" since I don't know your workflow).
msg376886 - (view) Author: Łukasz Langa (lukasz.langa) * (Python committer) Date: 2020-09-14 17:50
I will be cutting RC2 from the head of 3.9.
History
Date User Action Args
2020-09-14 17:50:17lukasz.langasetpriority: release blocker -> critical
status: open -> closed
stage: backport needed -> resolved
2020-09-14 17:50:02lukasz.langasetmessages: + msg376886
2020-09-11 13:39:30p-gansslesetmessages: + msg376730
2020-09-11 10:28:20vstinnersetmessages: + msg376712
2020-08-19 16:14:21vstinnersetmessages: + msg375657
2020-08-17 16:32:36p-gansslesetmessages: + msg375559
2020-08-17 07:47:38vstinnersetnosy: + vstinner
messages: + msg375530
2020-08-14 16:51:29p-gansslesetstage: patch review -> backport needed
2020-08-14 16:51:18p-gansslesetpriority: high -> release blocker
resolution: fixed
messages: + msg375419
2020-08-14 16:50:25p-gansslesetnosy: + lukasz.langa
messages: + msg375418
2020-08-14 15:18:45p-gansslesetmessages: + msg375412
2020-08-14 02:38:45miss-islingtonsetnosy: + miss-islington
pull_requests: + pull_request21001
2020-08-14 02:38:39p-gansslesetmessages: + msg375365
2020-06-18 14:41:21p-gansslesetkeywords: + patch
stage: needs patch -> patch review
pull_requests: + pull_request20143
2020-06-18 14:26:51p-gansslecreate