diff options
author | Maksim Levental <maksim.levental@gmail.com> | 2025-07-25 07:05:30 -0500 |
---|---|---|
committer | GitHub <noreply@github.com> | 2025-07-25 08:05:30 -0400 |
commit | 21774489f0a812254c110bebfff3aa9b6c4ad960 (patch) | |
tree | 39c1c243f40ef5793fcb509f4a7e57eef5af0061 /mlir/lib/Bindings/Python/IRAttributes.cpp | |
parent | 8005c6a1081c94e440b9c91e6b50cba9b72d186f (diff) | |
download | llvm-21774489f0a812254c110bebfff3aa9b6c4ad960.zip llvm-21774489f0a812254c110bebfff3aa9b6c4ad960.tar.gz llvm-21774489f0a812254c110bebfff3aa9b6c4ad960.tar.bz2 |
[mlir][python] fix PyDenseResourceElementsAttribute finalizer (#150561)
This PR melds https://github.com/llvm/llvm-project/pull/150137 and
https://github.com/llvm/llvm-project/pull/149414 *and* partially reverts
https://github.com/llvm/llvm-project/pull/124832.
The summary is the `PyDenseResourceElementsAttribute` finalizer/deleter
has/had two problems
1. wasn't threadsafe (can be called from a different thread than that
which currently holds the GIL)
2. can be called while the interpreter is "not initialized"
https://github.com/llvm/llvm-project/pull/124832 for some reason decides
to re-initialize the interpreter to avoid case 2 and runs afoul of the
fact that `Py_IsInitialized` can be false during the finalization of the
interpreter itself (e.g., at the end of a script).
I don't know why this decision was made (I missed the PR) but I believe
we should never be calling
[Py_Initialize](https://docs.python.org/3/c-api/init.html#c.Py_Initialize):
> In an application \*\*\*\***embedding Python**\*\*\*\*, this should be
called before using any other Python/C API functions
**but we aren't embedding Python**!
So therefore we will only be in case 2 when the interpreter is being
finalized and in that case we should just leak the buffer.
Note,
[lldb](https://github.com/llvm/llvm-project/blob/548ca9e97673a168023a616d311d901ca04b29a3/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp#L81-L93)
does a similar sort of thing for its finalizers.
Co-authored-by: Anton Korobeynikov <anton@korobeynikov.info>
Co-authored-by: Max Manainen <maximmanainen@gmail.com>
Co-authored-by: Anton Korobeynikov <anton@korobeynikov.info>
Co-authored-by: Max Manainen <maximmanainen@gmail.com>
Diffstat (limited to 'mlir/lib/Bindings/Python/IRAttributes.cpp')
-rw-r--r-- | mlir/lib/Bindings/Python/IRAttributes.cpp | 13 |
1 files changed, 10 insertions, 3 deletions
diff --git a/mlir/lib/Bindings/Python/IRAttributes.cpp b/mlir/lib/Bindings/Python/IRAttributes.cpp index 8f79caf..db84ee1 100644 --- a/mlir/lib/Bindings/Python/IRAttributes.cpp +++ b/mlir/lib/Bindings/Python/IRAttributes.cpp @@ -16,8 +16,8 @@ #include "NanobindUtils.h" #include "mlir-c/BuiltinAttributes.h" #include "mlir-c/BuiltinTypes.h" -#include "mlir/Bindings/Python/NanobindAdaptors.h" #include "mlir/Bindings/Python/Nanobind.h" +#include "mlir/Bindings/Python/NanobindAdaptors.h" #include "llvm/ADT/ScopeExit.h" #include "llvm/Support/raw_ostream.h" @@ -1428,6 +1428,12 @@ public: } }; +// Check if the python version is less than 3.13. Py_IsFinalizing is a part +// of stable ABI since 3.13 and before it was available as _Py_IsFinalizing. +#if PY_VERSION_HEX < 0x030d0000 +#define Py_IsFinalizing _Py_IsFinalizing +#endif + class PyDenseResourceElementsAttribute : public PyConcreteAttribute<PyDenseResourceElementsAttribute> { public: @@ -1474,8 +1480,9 @@ public: // The userData is a Py_buffer* that the deleter owns. auto deleter = [](void *userData, const void *data, size_t size, size_t align) { - if (!Py_IsInitialized()) - Py_Initialize(); + if (Py_IsFinalizing()) + return; + assert(Py_IsInitialized() && "expected interpreter to be initialized"); Py_buffer *ownedView = static_cast<Py_buffer *>(userData); nb::gil_scoped_acquire gil; PyBuffer_Release(ownedView); |