[all-commits] [llvm/llvm-project] 217744: [mlir][python] fix PyDenseResourceElementsAttribut...

Maksim Levental via All-commits all-commits at lists.llvm.org
Fri Jul 25 05:05:51 PDT 2025


  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: 21774489f0a812254c110bebfff3aa9b6c4ad960
      https://github.com/llvm/llvm-project/commit/21774489f0a812254c110bebfff3aa9b6c4ad960
  Author: Maksim Levental <maksim.levental at gmail.com>
  Date:   2025-07-25 (Fri, 25 Jul 2025)

  Changed paths:
    M mlir/lib/Bindings/Python/IRAttributes.cpp
    M mlir/test/python/ir/array_attributes.py

  Log Message:
  -----------
  [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 at korobeynikov.info>
Co-authored-by: Max Manainen <maximmanainen at gmail.com>

Co-authored-by: Anton Korobeynikov <anton at korobeynikov.info>
Co-authored-by: Max Manainen <maximmanainen at gmail.com>



To unsubscribe from these emails, change your notification settings at https://github.com/llvm/llvm-project/settings/notifications


More information about the All-commits mailing list