Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve the API/implementation for HPyCapsule_Destructor #377

Closed
antocuni opened this issue Nov 21, 2022 · 3 comments · Fixed by #378
Closed

Improve the API/implementation for HPyCapsule_Destructor #377

antocuni opened this issue Nov 21, 2022 · 3 comments · Fixed by #378
Assignees
Milestone

Comments

@antocuni
Copy link
Collaborator

this is best explained by this comment:
#308 (comment)

@fangerer fangerer self-assigned this Nov 22, 2022
@fangerer fangerer added this to the ABI version 1 milestone Nov 22, 2022
@fangerer
Copy link
Contributor

fangerer commented Nov 22, 2022

@antocuni I started to work on that and now remember some details: Even if we introduce a macro like you suggested, the problem is that there are the functions PyCapsule_GetDestructor and HPyCapsule_GetDestructor (note that capsule objects will be exchanged via Python modules and may be used from C API and HPy extensions). Those functions return the function pointer to the destructor. Now there are two cases:

  1. If the capsule object was created in an HPy extension:
    PyCapsule_GetDestructor returns the ptr to the trampoline.
    HPyCapsule_GetDestructor returns the ptr to the actual function impl.
  2. If the capsule object was created in a C API extension:
    PyCapsule_GetDestructor returns the ptr to the actual function impl..
    HPyCapsule_GetDestructor returns NULL (since we cannot provide a useful trampoline).

I can think of following solutions:

  1. Just don't provide HPyCapsule_GetDestructor.
  2. Always set a name in case the capsule is created from HPy. Then we can over-allocate the name and store the HPy destructor function pointer there.
  3. Create a HPyCapsule (see below) and don't use CPython's PyCapsule_New but implement a custom allocator. Then we just need to have a way to identify a capsule object is an HPy capsule such that we can do the cast and access the additional field. But that is possible.
typedef struct { 
    PyCapsule base; 
    HPyCapsule_Destructor hpy_dtor;
} HPyCapsule;

To be honest, I think we should just kill HPyCapsule_GetDestructor. What is that function good for? Why would you want to get the destructor of a foreign capsule object? If it is your own capsule object, you have the destructor anyway.

@fangerer
Copy link
Contributor

I just did a quick grep in the top4000 packages for PyCapsule_GetDestructor and didn't find a single usage. So, let's kill it!

@antocuni
Copy link
Collaborator Author

I just did a quick grep in the top4000 packages for PyCapsule_GetDestructor and didn't find a single usage. So, let's kill it!

yes, kill it!

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 a pull request may close this issue.

2 participants