Skip to content

Conversation

picnixz
Copy link
Member

@picnixz picnixz commented Sep 1, 2025

I'm not sure about kqueue as I can't check this myself. However note the following leak on main:

$ ./python -X showrefcount -c 'import select; s = select.poll(); t = type(s); t._do = s'
[74 refs, 43 blocks]

Instead of implementing the full GC protocol, we can make the types immutable.

Rationale for not backporting this: #138341 (comment).

@picnixz picnixz marked this pull request as draft September 1, 2025 18:56
@picnixz picnixz changed the title gh-116946: fully implement GC protocol for select.[e]poll gh-116946: correctly support GC for select.[e]poll Sep 2, 2025
@picnixz picnixz force-pushed the fix/gc/select-heap-types-116946 branch from 50a32c4 to 55c88e4 Compare September 2, 2025 08:32
@picnixz picnixz force-pushed the fix/gc/select-heap-types-116946 branch from 55c88e4 to 775f809 Compare September 2, 2025 08:33
@picnixz picnixz marked this pull request as ready for review September 2, 2025 08:35
@picnixz picnixz changed the title gh-116946: correctly support GC for select.[e]poll gh-116946: add Py_TPFLAGS_IMMUTABLETYPE to select.[e]poll Sep 2, 2025
Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

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

LGTM.

I don't know if "cannot create cycles as it only contains exact ints" comment is correct or not.

@picnixz
Copy link
Member Author

picnixz commented Sep 2, 2025

I don't know if "cannot create cycles as it only contains exact ints" comment is correct or not.

That's why I actually asked. The ints are constructed internally (AFAIU) so I that's why I didn't visit the dictionary. But I can visit it just to be sure.

@kumaraditya303 kumaraditya303 enabled auto-merge (squash) September 2, 2025 17:28
@kumaraditya303 kumaraditya303 merged commit 3ff00c7 into python:main Sep 2, 2025
45 checks passed
@picnixz picnixz deleted the fix/gc/select-heap-types-116946 branch September 2, 2025 18:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants