Ref leaks introduced by _io isolation (gh-101948)

[Eclips4]

Tried on current main.

./python -m test -R 3:3 test_nntplib
0:00:00 load avg: 2.49 Run tests sequentially
0:00:00 load avg: 2.49 [1/1] test_nntplib
beginning 6 repetitions
123456
......
test_nntplib leaked [1222, 1220, 1222] references, sum=3664
test_nntplib leaked [828, 827, 829] memory blocks, sum=2484
test_nntplib failed (reference leak)

== Tests result: FAILURE ==

1 test failed:
    test_nntplib

Total duration: 640 ms
Tests result: FAILURE

OS: WSL Ubuntu 20.04 & Windows 10

UPD:
Leaked tests:
test_nntplib
test_gzip
test_httpservers
test_xmlrpc
test_tarfile

Linked PRs

• GH-104510: Fix refleaks in _io base types #104516

[Eclips4]

Commit which introduced it
cc @erlend-aasland

[erlend-aasland]

Commit which introduced it

cc @erlend-aasland

Nice! We anticipated errors :) cc. @kumaraditya303 @vstinner

[Eclips4]

Commit which introduced it
cc @erlend-aasland

Nice! We anticipated errors :) cc. @kumaraditya303 @vstinner

However, nttplib deprecated since 3.11. So, it's good that it wasn't cut out in 3.12 or earlier😄

[Eclips4]

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

[sunmy2019]

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_data contains reference to self.

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=2484

to

test_nntplib leaked [343, 343, 343] references, sum=1029
test_nntplib leaked [207, 207, 208] memory blocks, sum=622

[Eclips4]

test_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

[sunmy2019]

@Eclips4 You can try doing similar things in #104457
186bf39 introduces three heap types; none have a *_clear set.

cpython/Modules/_io/_iomodule.c

Lines 681 to 687 in 186bf39

	// PyIOBase_Type subclasses
	ADD_TYPE(m, state->PyTextIOBase_Type, &textiobase_spec,
	state->PyIOBase_Type);
	ADD_TYPE(m, state->PyBufferedIOBase_Type, &bufferediobase_spec,
	state->PyIOBase_Type);
	ADD_TYPE(m, state->PyRawIOBase_Type, &rawiobase_spec,
	state->PyIOBase_Type);

But I have not carefully examined the correct value, and I am unsure whether this will fix the issue.

[Eclips4]

@Eclips4 You can try doing similar things in #104457 186bf39 introduces three heap types; none have a *_clear set.

cpython/Modules/_io/_iomodule.c

Lines 681 to 687 in 186bf39

	// PyIOBase_Type subclasses
	ADD_TYPE(m, state->PyTextIOBase_Type, &textiobase_spec,
	state->PyIOBase_Type);
	ADD_TYPE(m, state->PyBufferedIOBase_Type, &bufferediobase_spec,
	state->PyIOBase_Type);
	ADD_TYPE(m, state->PyRawIOBase_Type, &rawiobase_spec,
	state->PyIOBase_Type);

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.

[Eclips4]

Seems that implementing Py_tp_clear for textiobase_spec, bufferediobase_spec, rawiobase_spec doesn't make sense. There's no need in it. I'll check other types in iomodule_exec.

[erlend-aasland]

Bisected and confirmed that 186bf39 is the first bad commit.

[kumaraditya303]

#104516 fixes all the leaks mentioned

[kumaraditya303]

Thanks for the report.