Add test for gc being invoked in a thread finalizer (#365)

* PR replicates issue #362, and will trigger a pybind11 internal error using an un-patched version of pybind11
* Only run IWYU on files changed in PR

Note:
* This bug requires the code in question to be run in a thread created by C++.

fixes #362

Authors:
  - David Gardner (https://github.com/dagardner-nv)

Approvers:
  - Michael Demoret (https://github.com/mdemoret-nv)

URL: #365

python/mrc/tests/utils.cpp

@@ -21,6 +21,7 @@
 #include "mrc/version.hpp"
 
 #include <pybind11/cast.h>
+#include <pybind11/gil.h>  // for gil_scoped_acquire
 #include <pybind11/pybind11.h>
 
 #include <sstream>
@@ -30,6 +31,16 @@ namespace mrc::pytests {
 
 namespace py = pybind11;
 
+// Simple test class which uses pybind11's `gil_scoped_acquire` class in the destructor. Needed to repro #362
+struct RequireGilInDestructor
+{
+    ~RequireGilInDestructor()
+    {
+        // Grab the GIL
+        py::gil_scoped_acquire gil;
+    }
+};
+
 PYBIND11_MODULE(utils, py_mod)
 {
     py_mod.doc() = R"pbdoc()pbdoc";
@@ -48,6 +59,8 @@ PYBIND11_MODULE(utils, py_mod)
         },
         py::arg("msg") = "");
 
+    py::class_<RequireGilInDestructor>(py_mod, "RequireGilInDestructor").def(py::init<>());
+
     py_mod.attr("__version__") = MRC_CONCAT_STR(mrc_VERSION_MAJOR << "." << mrc_VERSION_MINOR << "."
                                                                   << mrc_VERSION_PATCH);
 }

python/tests/test_gil_tls.py

@@ -0,0 +1,45 @@
+# SPDX-FileCopyrightText: Copyright (c) 2023 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+# SPDX-License-Identifier: Apache-2.0
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+import threading
+
+import mrc
+from mrc.tests.utils import RequireGilInDestructor
+
+TLS = threading.local()
+
+
+def test_gil_thread_local_storage():
+    """
+    Test to reproduce issue #362
+    No asserts needed if it doesn't segfault, then we're good
+    """
+
+    def source_gen():
+        x = RequireGilInDestructor()
+        TLS.x = x
+        yield x
+
+    def init_seg(builder: mrc.Builder):
+        builder.make_source("souce_gen", source_gen)
+
+    pipe = mrc.Pipeline()
+    pipe.make_segment("seg1", init_seg)
+
+    options = mrc.Options()
+    executor = mrc.Executor(options)
+    executor.register_pipeline(pipe)
+    executor.start()
+    executor.join()