Skip to content

Conversation

@vstinner
Copy link
Member

@vstinnervstinner commented Aug 21, 2023

_socket.CAPI capsule contains a strong reference to _socket.socket type. The garbage collector cannot visit this reference and so cannot create the reference cycle involving _socket.socket (a heap type creates a reference cycle to inside, via MRO and methods). At Python, _PyImport_ClearModules() sets _socket attributes to None which works around the issue.

If the module is cleared from sys.modules manually, _PyImport_ClearModules() cannot set _socket.CAPI to None and so the issue cannot be worked around.

Change _socket.CAPI to use a borrowed reference instead of a strong reference to allow clearing _socket.socket in this case.

_socket.CAPI capsule contains a strong reference to _socket.socket type. The garbage collector cannot visit this reference and so cannot create the reference cycle involving _socket.socket (a heap type creates a reference cycle to inside, via MRO and methods). At Python, _PyImport_ClearModules() sets _socket attributes to None which works around the issue. If the module is cleared from sys.modules manually, _PyImport_ClearModules() cannot set _socket.CAPI to None and so the issue cannot be worked around. Change _socket.CAPI to use a borrowed reference instead of a strong reference to allow clearing _socket.socket in this case. Co-authored-by: Kirill Podoprigora <kirill.bast9@mail.ru>
@vstinner
Copy link
MemberAuthor

Storing a borrowed reference in the capsule object is unsafe if the capsule is used after the extension is finalized. But right now, it's not possible to configure a capsule object to visit its strong references.

@vstinner
Copy link
MemberAuthor

If we agree that this approach is the correct one, I will write a NEWS entry and backport the change to stable versions.

@vstinner
Copy link
MemberAuthor

The PR fix the issue. Example with reproducer attached to the issue:

$ ./python -X showrefcount script.py exit [0 refs, 0 blocks] 

@vstinner
Copy link
MemberAuthor

Since the _datetime capsule also uses borrowed references to types, and the _datetime capsule (C API) is more likely to be used in the wild than the _socket capsule (C API), since there is a dedicated public datetime.h C API using it. IMO it's acceptable to use borrowed references in the _socket capsule (C API).

@vstinner
Copy link
MemberAuthor

I merged the safe approach instead: PR #108339.

@vstinnervstinner deleted the sock_capsule branch August 23, 2023 22:20
Sign up for freeto 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.

2 participants

@vstinner@bedevere-bot