Skip to content

Commit

Permalink
Pare back test to just the minimum, as this doesn't in reality need t…
Browse files Browse the repository at this point in the history
…o call gc.collect
  • Loading branch information
dagardner-nv committed Aug 28, 2023
1 parent 3e6e466 commit 785e983
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 15 deletions.
19 changes: 8 additions & 11 deletions python/mrc/tests/utils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,31 +20,28 @@
#include "mrc/utils/string_utils.hpp"
#include "mrc/version.hpp"

#include <glog/logging.h>
#include <pybind11/cast.h>
#include <pybind11/gil.h> // for gil_scoped_acquire
#include <pybind11/pybind11.h>
#include <pybind11/pytypes.h>

#include <array> // std::array needed for py::print
#include <sstream>
#include <stdexcept>

namespace mrc::pytests {

namespace py = pybind11;

// Simple test class which invokes Python's GC. Needed to repro #
struct ObjCallingGC
// Simple test class which uses pybind11's `gil_scoped_acquire` class in the destructor. Needed to repro #362
struct RequireGilInDestructor
{
ObjCallingGC() = default;

static void finalize()
~RequireGilInDestructor()
{
LOG(INFO) << "ObjCallingGC::finalize()";
// Grab the GIL to print
py::gil_scoped_acquire gil;
py::object gc = py::module::import("gc");
LOG(INFO) << "ObjCallingGC::finalize() - calling collect";
gc.attr("collect")();

py::print("RequireGilInDestructor::~RequireGilInDestructor() - calling destructor");
}
};

Expand All @@ -66,7 +63,7 @@ PYBIND11_MODULE(utils, py_mod)
},
py::arg("msg") = "");

py::class_<ObjCallingGC>(py_mod, "ObjCallingGC").def(py::init<>()).def_static("finalize", &ObjCallingGC::finalize);
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);
Expand Down
6 changes: 2 additions & 4 deletions python/tests/test_gil_tls.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,9 @@
# limitations under the License.

import threading
import weakref

import mrc
from mrc.tests.utils import ObjCallingGC
from mrc.tests.utils import RequireGilInDestructor

TLS = threading.local()

Expand All @@ -31,8 +30,7 @@ def test_gc_called_in_thread_finalizer():

def source_gen():
mrc.logging.log("source_gen")
x = ObjCallingGC()
weakref.finalize(x, x.finalize)
x = RequireGilInDestructor()
TLS.x = x
yield x

Expand Down

0 comments on commit 785e983

Please sign in to comment.