Skip to content

Commit 874b74b

Browse files
maksleventalaslmamanain
authored
[mlir][python] fix PyDenseResourceElementsAttribute finalizer (#150561)
This PR melds llvm/llvm-project#150137 and llvm/llvm-project#149414 *and* partially reverts llvm/llvm-project#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" llvm/llvm-project#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 <[email protected]> Co-authored-by: Max Manainen <[email protected]> Co-authored-by: Anton Korobeynikov <[email protected]> Co-authored-by: Max Manainen <[email protected]>
1 parent 6952074 commit 874b74b

File tree

1 file changed

+10
-3
lines changed

1 file changed

+10
-3
lines changed

mlir/lib/Bindings/Python/IRAttributes.cpp

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,8 @@
1616
#include "NanobindUtils.h"
1717
#include "mlir-c/BuiltinAttributes.h"
1818
#include "mlir-c/BuiltinTypes.h"
19-
#include "mlir/Bindings/Python/NanobindAdaptors.h"
2019
#include "mlir/Bindings/Python/Nanobind.h"
20+
#include "mlir/Bindings/Python/NanobindAdaptors.h"
2121
#include "llvm/ADT/ScopeExit.h"
2222
#include "llvm/Support/raw_ostream.h"
2323

@@ -1428,6 +1428,12 @@ class PyDenseIntElementsAttribute
14281428
}
14291429
};
14301430

1431+
// Check if the python version is less than 3.13. Py_IsFinalizing is a part
1432+
// of stable ABI since 3.13 and before it was available as _Py_IsFinalizing.
1433+
#if PY_VERSION_HEX < 0x030d0000
1434+
#define Py_IsFinalizing _Py_IsFinalizing
1435+
#endif
1436+
14311437
class PyDenseResourceElementsAttribute
14321438
: public PyConcreteAttribute<PyDenseResourceElementsAttribute> {
14331439
public:
@@ -1474,8 +1480,9 @@ class PyDenseResourceElementsAttribute
14741480
// The userData is a Py_buffer* that the deleter owns.
14751481
auto deleter = [](void *userData, const void *data, size_t size,
14761482
size_t align) {
1477-
if (!Py_IsInitialized())
1478-
Py_Initialize();
1483+
if (Py_IsFinalizing())
1484+
return;
1485+
assert(Py_IsInitialized() && "expected interpreter to be initialized");
14791486
Py_buffer *ownedView = static_cast<Py_buffer *>(userData);
14801487
nb::gil_scoped_acquire gil;
14811488
PyBuffer_Release(ownedView);

0 commit comments

Comments
 (0)