Repository navigation
Ref leaks introduced by _io isolation (gh-101948) #104510
Description
Activity
Commit which introduced it
cc @erlend-aaslandReacted by Erlend E. AaslandCommit which introduced it
Nice! We anticipated errors :) cc. @kumaraditya303 @vstinner
Commit which introduced it
cc @erlend-aaslandNice! We anticipated errors :) cc. @kumaraditya303 @vstinner
However,
nttplibdeprecated since 3.11. So, it's good that it wasn't cut out in 3.12 or earlier😄test_gzip also affected by this commit:
./python -m test -R 3:3 test_gzip 0:00:00 load avg: 0.00 Run tests sequentially 0:00:00 load avg: 0.00 [1/1] test_gzip beginning 6 repetitions 123456 ...... test_gzip leaked [4307, 4303, 4307] references, sum=12917 test_gzip leaked [3239, 3237, 3239] memory blocks, sum=9715 test_gzip failed (reference leak) == Tests result: FAILURE == 1 test failed: test_gzip Total duration: 4.0 sec Tests result: FAILURE
Incorrectly implemented GC likely causes this.
e.g. I tracked down one cyclic reference here:
class MockedNNTPTestsMixin: # Override in derived classes handler_class = None def setUp(self): super().setUp() self.make_server() def tearDown(self): super().tearDown() + self.handler._push_data = None del self.server def make_server(self, *args, **kwargs): self.handler = self.handler_class() self.sio, file = make_mock_file(self.handler) self.server = NNTPServer(file, 'test.server', *args, **kwargs) return self.server
self.handler._push_datacontains reference toself.When I add the above line, ref leaks drop from
test_nntplib leaked [1222, 1220, 1222] references, sum=3664 test_nntplib leaked [828, 827, 829] memory blocks, sum=2484to
test_nntplib leaked [343, 343, 343] references, sum=1029 test_nntplib leaked [207, 207, 208] memory blocks, sum=622test_httpservers, test_xmlrpc, test_tarfile cause also cause reference leaks.
So, we have at least five tests with reference leaks:- test_nntplib
- test_gzip
- test_httpservers
- test_xmlrpc
- test_tarfile
@Eclips4 You can try doing similar things in #104457
186bf39 introduces three heap types; none have a*_clearset.
cpython/Modules/_io/_iomodule.c
Lines 681 to 687 in 186bf39
But I have not carefully examined the correct value, and I am unsure whether this will fix the issue.
- changed the title
[-]Some tests are leaked[/-][+]Ref leaks introduced by 186bf39[/+]on May 15, 2023 @Eclips4 You can try doing similar things in #104457 186bf39 introduces three heap types; none have a
*_clearset.cpython/Modules/_io/_iomodule.c
Lines 681 to 687 in 186bf39
But I have not carefully examined the correct value, and I am unsure whether this will fix the issue.
Sure! I can try this, and later let you know results.
Seems that implementing
Py_tp_clearfortextiobase_spec,bufferediobase_spec,rawiobase_specdoesn't make sense. There's no need in it. I'll check other types iniomodule_exec.Bisected and confirmed that 186bf39 is the first bad commit.
Reacted by Kirill Podoprigora#104516 fixes all the leaks mentioned
Reacted by sunmy2019 and Kirill Podoprigora- changed the title
[-]Ref leaks introduced by 186bf39[/-][+]Ref leaks introduced by _io isolation (gh-101948)[/+]on May 16, 2023 - linked a pull request that will close this issueGH-104510: Fix refleaks in _io base types #104516
on May 16, 2023 - moved this from Todo to Done in Release and Deferred blockers 🚫
on May 16, 2023 Thanks for the report.
- added 2 commits that reference this issue
on May 16, 2023
Tried on current main.
OS: WSL Ubuntu 20.04 & Windows 10
UPD:
Leaked tests:
test_nntplib
test_gzip
test_httpservers
test_xmlrpc
test_tarfile
Linked PRs