Ref leaks introduced by _io isolation (gh-101948)

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

@erlend-aasland

Commit which introduced it
cc @erlend-aasland

@erlend-aasland

Commit which introduced it

cc @erlend-aasland

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

@erlend-aasland

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😄

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_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

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

@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.

@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.

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.

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

#104516 fixes all the leaks mentioned

GH-104510: Fix refleaks in `_io` base types (#104516)

Thanks for the report.

Merge branch 'main' into supermon

Merge branch 'main' into 709funconly

Navigation Menu

Repository navigation

Ref leaks introduced by _io isolation (gh-101948) #104510

Linked PRs

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

	// 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);

Ref leaks introduced by _io isolation (gh-101948) #104510

Description

Linked PRs

Activity

Eclips4 commented on May 15, 2023

erlend-aasland commented on May 15, 2023

Eclips4 commented on May 15, 2023

Eclips4 commented on May 15, 2023

sunmy2019 commented on May 15, 2023

Eclips4 commented on May 15, 2023

sunmy2019 commented on May 15, 2023

Eclips4 commented on May 15, 2023

Eclips4 commented on May 15, 2023

erlend-aasland commented on May 15, 2023

kumaraditya303 commented on May 15, 2023

kumaraditya303 commented on May 16, 2023

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions