Skip to content

gh-108240: Fix a reference cycle in _socket.CAPI capsule - #108241

Closed
vstinner wants to merge 1 commit into
python:mainfrom
vstinner:sock_capsule
Closed

gh-108240: Fix a reference cycle in _socket.CAPI capsule#108241
vstinner wants to merge 1 commit into
python:mainfrom
vstinner:sock_capsule

Conversation

@vstinner

@vstinnervstinner commented Aug 21, 2023

Copy link
Copy Markdown
Member

_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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
MemberAuthor

I merged the safe approach instead: PR #108339.

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