[Mlir-commits] [mlir] [mlir][python] fix PyDenseResourceElementsAttribute finalizer (PR #150561)
Maksim Levental
llvmlistbot at llvm.org
Thu Jul 24 18:21:28 PDT 2025
https://github.com/makslevental created https://github.com/llvm/llvm-project/pull/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 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 when the interpreter is being finalized and in that case we should just speak 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 **but lldb does actually embed Python!**
>From 18d6038b7129bdbdca56f2570ca208f3c3d430cc Mon Sep 17 00:00:00 2001
From: Maksim Levental <maksim.levental at gmail.com>
Date: Thu, 24 Jul 2025 20:52:26 -0400
Subject: [PATCH] [mlir][python] fix PyDenseResourceElementsAttribute finalizer
---
mlir/lib/Bindings/Python/IRAttributes.cpp | 10 ++++++++--
mlir/test/python/ir/array_attributes.py | 22 ++++++++++++++++++++--
2 files changed, 28 insertions(+), 4 deletions(-)
diff --git a/mlir/lib/Bindings/Python/IRAttributes.cpp b/mlir/lib/Bindings/Python/IRAttributes.cpp
index 8f79caf08a6d0..585b2317dde2d 100644
--- a/mlir/lib/Bindings/Python/IRAttributes.cpp
+++ b/mlir/lib/Bindings/Python/IRAttributes.cpp
@@ -1428,6 +1428,12 @@ class PyDenseIntElementsAttribute
}
};
+// 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,8 @@ class PyDenseResourceElementsAttribute
// 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;
Py_buffer *ownedView = static_cast<Py_buffer *>(userData);
nb::gil_scoped_acquire gil;
PyBuffer_Release(ownedView);
diff --git a/mlir/test/python/ir/array_attributes.py b/mlir/test/python/ir/array_attributes.py
index ef1d835fc6401..a2c359a022f8b 100644
--- a/mlir/test/python/ir/array_attributes.py
+++ b/mlir/test/python/ir/array_attributes.py
@@ -31,6 +31,7 @@ def testGetDenseElementsUnsupported():
# CHECK: unimplemented array format conversion from format:
print(e)
+
# CHECK-LABEL: TEST: testGetDenseElementsUnSupportedTypeOkIfExplicitTypeProvided
@run
def testGetDenseElementsUnSupportedTypeOkIfExplicitTypeProvided():
@@ -41,8 +42,9 @@ def testGetDenseElementsUnSupportedTypeOkIfExplicitTypeProvided():
# realistic example would be a NumPy extension type like the bfloat16
# type from the ml_dtypes package, which isn't a dependency of this
# test.
- attr = DenseElementsAttr.get(array.view(np.datetime64),
- type=IntegerType.get_signless(64))
+ attr = DenseElementsAttr.get(
+ array.view(np.datetime64), type=IntegerType.get_signless(64)
+ )
# CHECK: dense<{{\[}}[1, 2, 3], [4, 5, 6]]> : tensor<2x3xi64>
print(attr)
# CHECK: {{\[}}[1 2 3]
@@ -135,6 +137,7 @@ def testGetDenseElementsFromListMixedTypes():
# Splats.
################################################################################
+
# CHECK-LABEL: TEST: testGetDenseElementsSplatInt
@run
def testGetDenseElementsSplatInt():
@@ -617,3 +620,18 @@ def test_attribute(context, mview):
# CHECK: BACKING MEMORY DELETED
# CHECK: EXIT FUNCTION
print("EXIT FUNCTION")
+
+
+# CHECK-LABEL: TEST: testDanglingResource
+print("TEST: testDanglingResource")
+# see https://github.com/llvm/llvm-project/pull/149414, https://github.com/llvm/llvm-project/pull/150137
+# This error occurs only when there is an alive context with a DenseResourceElementsAttr
+# in the end of the program, so we put it here without an encapsulating function.
+ctx = Context()
+
+with ctx, Location.unknown():
+ DenseResourceElementsAttr.get_from_buffer(
+ memoryview(np.array([1, 2, 3])),
+ "some_resource",
+ RankedTensorType.get((3,), IntegerType.get_signed(32)),
+ )
More information about the Mlir-commits
mailing list