Skip to content

test_datetime leaks references #119655

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

Closed
vstinner opened this issue May 28, 2024 · 5 comments
Closed

test_datetime leaks references #119655

vstinner opened this issue May 28, 2024 · 5 comments

Comments

@vstinner
Copy link
Member

vstinner commented May 28, 2024

Example:

$ ./python -m test -R 3:3 test_datetime -m test.datetimetester.TestDateTime_Pure.test_compat_unpickle
(...)
test_datetime leaked [8, 8, 8] references, sum=24
test_datetime leaked [8, 8, 8] memory blocks, sum=24

Regression: commit 3e8b609

commit 3e8b60905e97a4fe89bb24180063732214368938
Author: Erlend E. Aasland <erlend@python.org>
Date:   Tue May 28 00:02:46 2024 +0200

    gh-117398: Add multiphase support to _datetime (gh-119373)
    
    This is minimal support.  Subinterpreters are not supported yet.  That will be addressed in a later change.
    
    Co-authored-by: Eric Snow <ericsnowcurrently@gmail.com>

 Lib/test/datetimetester.py | 21 +++++++++++++++++++++
 Modules/_datetimemodule.c  | 26 +++++++++++---------------
 2 files changed, 32 insertions(+), 15 deletions(-)

cc @erlend-aasland @ericsnowcurrently

Linked PRs

@encukou
Copy link
Member

encukou commented May 28, 2024

I'm looking into it.

@encukou
Copy link
Member

encukou commented May 28, 2024

See: #119662

encukou added a commit that referenced this issue May 28, 2024
GH-119662)

- While datetime uses global state, only initialize it once.
- While `capi` is static, don't free it (thanks @neonene in https://github.com/python/cpython/pull/119641/files#r1616710048)
ericsnowcurrently pushed a commit to ericsnowcurrently/cpython that referenced this issue May 28, 2024
…don't free it (pythonGH-119662)

- While datetime uses global state, only initialize it once.
- While `capi` is static, don't free it (thanks @neonene in https://github.com/python/cpython/pull/119641/files#r1616710048)
@erlend-aasland
Copy link
Contributor

Thanks for fixing this, Petr!

@Eclips4
Copy link
Member

Eclips4 commented May 29, 2024

Unfortunately, there's a new leaks in the test_datetime:

./python.exe -m test -R 3:3 test_datetime
Using random seed: 1167096647
0:00:00 load avg: 15.47 Run 1 test sequentially
0:00:00 load avg: 15.47 [1/1] test_datetime
beginning 6 repetitions. Showing number of leaks (. for 0 or less, X for 10 or more)
123:456
XXX XXX
test_datetime leaked [39, 39, 39] references, sum=117
test_datetime leaked [29, 29, 29] memory blocks, sum=87
test_datetime failed (reference leak)

== Tests result: FAILURE ==

1 test failed:
    test_datetime

Total duration: 26.5 sec
Total tests: run=972 skipped=8
Total test files: run=1/1 failed=1
Result: FAILURE

Bisected to the 548a11d

@ericsnowcurrently
Copy link
Member

Sorry about the leaks, all. I'll be extra careful from here on out.

@Eclips4 Eclips4 closed this as completed May 30, 2024
noahbkim pushed a commit to hudson-trading/cpython that referenced this issue Jul 11, 2024
estyxx pushed a commit to estyxx/cpython that referenced this issue Jul 17, 2024
…don't free it (pythonGH-119662)

- While datetime uses global state, only initialize it once.
- While `capi` is static, don't free it (thanks @neonene in https://github.com/python/cpython/pull/119641/files#r1616710048)
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

No branches or pull requests

5 participants