Require static memory allocation for PyCapsule names #300
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR fixes a technical overlook. According to the Python documentation, the only requirement for PyCapsule names is that it must outlive the capsule, but it can be either statically or dynamically allocated. For the latter, the suggestion (not requirement) is to free it when the destructor is launched.
This brings in two issues. First, the DLPack protocol requires the consumer to overwrite the name, but if the name is dynamically allocated, the following operation would raise a compiler warning to the very least:
A further problem is we do not know if
free
is the correct deallocation routine, which leads to potential crash hazard.The second problem is a symmetric one w.r.t the 1st: if the consumer also allocates a name dynamically, by the time of destruction it could be freed by the producer's (mismatching) deallocation routine.
This PR suggests a simple fix by asking all libraries to allocate the names statically. This has no practical impact because it's already the status quo, at least in CuPy and PyTorch.