Skip to content

Conversation

@shino16
Copy link
Collaborator

@shino16 shino16 commented Nov 18, 2025

This PR makes _call_dispatch use __get__(slf, type(slf)) instead of __get__(slf), working around a CPython segfault (python/cpython#132747).

This will eliminate the segmentation fault mentioned in #2677 (comment). From Python 3.13.3 this workaround will no longer be necessary, but I think we can keep it anyway.

Copy link
Collaborator

@beverlylytle beverlylytle left a comment

Choose a reason for hiding this comment

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

🎉

@shino16 shino16 requested a review from Copilot November 18, 2025 22:59
Copilot finished reviewing on behalf of shino16 November 18, 2025 23:00
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR addresses a CPython segfault issue (python/cpython#132747) by correctly calling the descriptor protocol's __get__ method with both required arguments. The change ensures that when unbinding builtin methods, the type argument is explicitly passed to __get__.

  • Modified the descriptor __get__ call to include the type argument: __get__(slf, type(slf)) instead of __get__(slf)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Collaborator

@kshitij12345 kshitij12345 left a comment

Choose a reason for hiding this comment

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

Great, thanks @shino16

Copy link
Collaborator

@riccardofelluga riccardofelluga left a comment

Choose a reason for hiding this comment

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

Way to go! Great catch!

For info, what changes under the hood when we give the type to the get method?

@shino16
Copy link
Collaborator Author

shino16 commented Nov 20, 2025

Thanks! I believe there is no change. The official doc says

If a descriptor is found for a.x, then it is invoked with: desc.__get__(a, type(a)).

A dotted lookup such as super(A, obj).m searches obj.__class__.__mro__ for the base class B immediately following A and then returns B.__dict__['m'].__get__(obj, A). If not a descriptor, m is returned unchanged.

(copied from #2677 (comment))

@shino16
Copy link
Collaborator Author

shino16 commented Nov 20, 2025

@KaelanDt This is ready for your review. Thanks!

Sign up for free to 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.

4 participants