MAINT: Ensure raw dlpack deleter works when called without the GIL #22510
Conversation
charris
changed the title
| // PyObjects stored in them after the Python runtime has already been | ||
| // terminated. | ||
| if (!Py_IsInitialized()) | ||
| return; |
There was a problem hiding this comment.
Please add brackets (slight preference for /* style comments also).
Curious, is this really expected or more of a hack for programs which don't ensure cleanup has a well defined order?
There was a problem hiding this comment.
Curious, is this really expected or more of a hack for programs which don't ensure cleanup has a well defined order?
This is a real problem in PyTorch. In particular, we do have quite a few long-lived c++ objects. And because C++ cleanup order is not really guaranteed, it is possible for such a long lived c++ object to hold onto a pyobject. We had quite a few deadlocks because of it pytorch/pytorch#73961 and pytorch/pytorch#57488 are example of cleanups we did there.
There was a problem hiding this comment.
I can remove this if you prefer, but given that our Tensor object is potentially one such c++ owned object and it will call this function on destruction. I would prefer to keep it here.
There was a problem hiding this comment.
Let's keep it indefinitely... It has a bit of a code smell to me, but it's localized enough.
It is annoying to test this (and I guess may be indirectly tested soon at least); so plan to make an exception here and merge it once the test suite is done.
Thanks.
|
@seberg I don't plan on releasing a 1.23.6, not least because parts of the release code will break sometime in December when macos 10.15 goes away. |
3 participants
Fix for #22507
This makes this deleter compliant with the proposal at dmlc/dlpack#103