[Mlir-commits] [mlir] Added FT lock around PyOperation valid flag to prevent a data race (PR #126709)

[Peter Hawkins]

hawkinsp wrote:

Hmm. Something is strange to me here.

In your race report, T52 is calling the destructor (`~PyOperation`). But shouldn't that be impossible? T57 should hold a reference count via `PyOperationIterator::dunderNext()` here, via the `PyOperationRef`: https://github.com/llvm/llvm-project/blob/76392421553f3b25552970812868f70721971451/mlir/lib/Bindings/Python/IRCore.cpp#L501


For those reading along, the race report says:
```
  Write of size 1 at 0x7fffc80b00b9 by thread T52:
    #0 mlir::python::PyOperation::setInvalid() /project/llvm-project/mlir/lib/Bindings/Python/IRModule.h:703:29 (_mlir.cpython-313t-x86_64-linux-gnu.so+0xb7b98) (BuildId: 3dc56c655ac19086)
    #1 mlir::python::PyMlirContext::clearOperation(MlirOperation) /project/llvm-project/mlir/lib/Bindings/Python/IRCore.cpp:724:10 (_mlir.cpython-313t-x86_64-linux-gnu.so+0xb7b98)
    #2 mlir::python::PyOperation::~PyOperation() /project/llvm-project/mlir/lib/Bindings/Python/IRCore.cpp:1196:19 (_mlir.cpython-313t-x86_64-linux-gnu.so+0xbb6e0) (BuildId: 3dc56c655ac19086)
    #3 void nanobind::detail::wrap_destruct<mlir::python::PyOperation>(void*) /tmp/cpython-tsan/lib/python3.13t/site-packages/nanobind/include/nanobind/nb_class.h:245:21 (_mlir.cpython-313t-x86_64-linux-gnu.so+0xe767b) (BuildId: 3dc56c655ac19086)
    #4 nanobind::detail::inst_dealloc(_object*) /tmp/cpython-tsan/lib/python3.13t/site-packages/nanobind/src/nb_type.cpp:255:13 (_mlir.cpython-313t-x86_64-linux-gnu.so+0x13db27) (BuildId: 3dc56c655ac19086)
    ...
    
  Previous read of size 1 at 0x7fffc80b00b9 by thread T57:
    #0 mlir::python::PyOperation::checkValid() const /project/llvm-project/mlir/lib/Bindings/Python/IRCore.cpp:1283:8 (_mlir.cpython-313t-x86_64-linux-gnu.so+0xbc1f1) (BuildId: 3dc56c655ac19086)
    #1 mlir::python::PyOperation::createOpView() /project/llvm-project/mlir/lib/Bindings/Python/IRCore.cpp:1607:3 (_mlir.cpython-313t-x86_64-linux-gnu.so+0xbf02f) (BuildId: 3dc56c655ac19086)
    #2 (anonymous namespace)::PyOperationIterator::dunderNext() /project/llvm-project/mlir/lib/Bindings/Python/IRCore.cpp:504:29 (_mlir.cpython-313t-x86_64-linux-gnu.so+0xd3752) (BuildId: 3dc56c655ac19086)
    #3 void nanobind::cpp_function_def<(anonymous namespace)::PyOperationIterator, nanobind::object, (anonymous namespace)::PyOperationIterator, nanobind::scope, nanobind::name, nanobind::is_method>(nanobind::object ((anonymous namespace)::PyOperationIterator::*)(), nanobind::scope const&, nanobind::name const&, nanobind::is_method const&)::'lambda'((anonymous namespace)::PyOperationIterator*)::operator()((anonymous namespace)::PyOperationIterator*) const /tmp/cpython-tsan/lib/python3.13t/site-packages/nanobind/include/nanobind/nb_func.h:376:20 (_mlir.cpython-313t-x86_64-linux-gnu.so+0xd3ace) (BuildId: 3dc56c655ac19086)
    #4 _object* nanobind::detail::func_create<false, true, void nanobind::cpp_function_def<(anonymous namespace)::PyOperationIterator, nanobind::object, (anonymous namespace)::PyOperationIterator, nanobind::scope, nanobind::name, nanobind::is_method>(nanobind::object ((anonymous namespace)::PyOperationIterator::*)(), nanobind::scope const&, nanobind::name const&, nanobind::is_method const&)::'lambda'((anonymous namespace)::PyOperationIterator*), nanobind::object, (anonymous namespace)::PyOperationIterator*, 0ul, nanobind::scope, nanobind::name, nanobind::is_method>((anonymous namespace)::PyOperationIterator&&, nanobind::object (*)(nanobind::scope, nanobind::name, nanobind::is_method), std::integer_sequence<unsigned long, 0ul>, nanobind::scope const&, nanobind::name const&, nanobind::is_method const&)::'lambda'(void*, _object**, unsigned char*, nanobind::rv_policy, nanobind::detail::cleanup_list*)::operator()(void*, _object**, unsigned char*, nanobind::rv_policy, nanobind::detail::cleanup_list*) const /tmp/cpython-tsan/lib/python3.13t/site-packages/nanobind/include/nanobind/nb_func.h:275:24 (_mlir.cpython-313t-x86_64-linux-gnu.so+0xd3ace)
    ...
```

https://github.com/llvm/llvm-project/pull/126709