Skip to content

Conversation

@erlend-aasland
Copy link
Contributor

@erlend-aaslanderlend-aasland commented Mar 28, 2023

@erlend-aaslanderlend-aasland added the 🔨 test-with-refleak-buildbots Test PR w/ refleak buildbots; report in status section label Apr 4, 2023
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @erlend-aasland for commit b4edf64 🤖

If you want to schedule another build, you need to add the 🔨 test-with-refleak-buildbots label again.

@bedevere-botbedevere-bot removed the 🔨 test-with-refleak-buildbots Test PR w/ refleak buildbots; report in status section label Apr 4, 2023
@erlend-aasland
Copy link
ContributorAuthor

There's some leaks that needs to be adressed:

$ ./python.exe measure.pybefore=46451, after=47809before=47811, after=48549before=48549, after=49287before=49287, after=50025before=50025, after=50763
measure.py
importgcimportsysfor_inrange(5): gc.collect() before=sys.gettotalrefcount() import_socketdelsys.modules["_socket"] del_socketgc.collect() after=sys.gettotalrefcount() print(f"{before=}, {after=}") # assert after == before

... and these two:

$ ./python.exe -m test -R : test_socket -m RecvmsgIntoSCMRightsStreamTest Raised RLIMIT_NOFILE: 256 -> 10240:00:00 load avg: 1.96 Run tests sequentially0:00:00 load avg: 1.96 [1/1] test_socketbeginning 9 repetitions123456789.........test_socket leaked [10, 10, 10, 10] file descriptors, sum=40test_socket failed (reference leak)== Tests result: FAILURE ==1 test failed: test_socketTotal duration: 1.8 secTests result: FAILURE $ ./python.exe -m test -R : test_socket -m RecvmsgSCMRightsStreamTest Raised RLIMIT_NOFILE: 256 -> 10240:00:00 load avg: 2.52 Run tests sequentially0:00:00 load avg: 2.52 [1/1] test_socketbeginning 9 repetitions123456789.........test_socket leaked [10, 10, 10, 10] file descriptors, sum=40test_socket failed (reference leak)== Tests result: FAILURE ==1 test failed: test_socketTotal duration: 1.8 secTests result: FAILURE

@erlend-aasland
Copy link
ContributorAuthor

Regarding #103094 (comment): the file descriptors leak on main as well, so it is not a regression introduced by this PR. Let's not bother with those. There's still references leaked by repeated init of the module, though.

@erlend-aasland
Copy link
ContributorAuthor

Seems like the leaks are due to the capsulated C API not being freed.

@erlend-aasland
Copy link
ContributorAuthor

I'd like to hold this PR until #103261 has landed.

@erlend-aaslanderlend-aasland marked this pull request as ready for review April 8, 2023 19:39
@erlend-aaslanderlend-aasland added the 🔨 test-with-refleak-buildbots Test PR w/ refleak buildbots; report in status section label Apr 8, 2023
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @erlend-aasland for commit 2019a93 🤖

If you want to schedule another build, you need to add the 🔨 test-with-refleak-buildbots label again.

@bedevere-botbedevere-bot removed the 🔨 test-with-refleak-buildbots Test PR w/ refleak buildbots; report in status section label Apr 8, 2023
Copy link
Contributor

@kumaraditya303kumaraditya303 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@kumaraditya303kumaraditya303 merged commit f329a8b into python:mainApr 9, 2023
@kumaraditya303kumaraditya303 deleted the isolate-socket branch April 9, 2023 01:03
warsaw pushed a commit to warsaw/cpython that referenced this pull request Apr 11, 2023
@CharlieZhao95CharlieZhao95 mentioned this pull request Apr 12, 2023
5 tasks
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@erlend-aasland@bedevere-bot@kumaraditya303