Commit 3d78346
[mlir][python] fix PyDenseResourceElementsAttribute finalizer (llvm#150561)
This PR melds llvm#150137 and
llvm#149414 *and* partially reverts
llvm#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#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 82ac868 commit 3d78346
File tree
2 files changed
+30
-5
lines changed- mlir
- lib/Bindings/Python
- test/python/ir
2 files changed
+30
-5
lines changed| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
16 | 16 | | |
17 | 17 | | |
18 | 18 | | |
19 | | - | |
20 | 19 | | |
| 20 | + | |
21 | 21 | | |
22 | 22 | | |
23 | 23 | | |
| |||
1428 | 1428 | | |
1429 | 1429 | | |
1430 | 1430 | | |
| 1431 | + | |
| 1432 | + | |
| 1433 | + | |
| 1434 | + | |
| 1435 | + | |
| 1436 | + | |
1431 | 1437 | | |
1432 | 1438 | | |
1433 | 1439 | | |
| |||
1474 | 1480 | | |
1475 | 1481 | | |
1476 | 1482 | | |
1477 | | - | |
1478 | | - | |
| 1483 | + | |
| 1484 | + | |
| 1485 | + | |
1479 | 1486 | | |
1480 | 1487 | | |
1481 | 1488 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
31 | 31 | | |
32 | 32 | | |
33 | 33 | | |
| 34 | + | |
34 | 35 | | |
35 | 36 | | |
36 | 37 | | |
| |||
41 | 42 | | |
42 | 43 | | |
43 | 44 | | |
44 | | - | |
45 | | - | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
46 | 48 | | |
47 | 49 | | |
48 | 50 | | |
| |||
135 | 137 | | |
136 | 138 | | |
137 | 139 | | |
| 140 | + | |
138 | 141 | | |
139 | 142 | | |
140 | 143 | | |
| |||
617 | 620 | | |
618 | 621 | | |
619 | 622 | | |
| 623 | + | |
| 624 | + | |
| 625 | + | |
| 626 | + | |
| 627 | + | |
| 628 | + | |
| 629 | + | |
| 630 | + | |
| 631 | + | |
| 632 | + | |
| 633 | + | |
| 634 | + | |
| 635 | + | |
| 636 | + | |
| 637 | + | |
0 commit comments