From 03b772817ff776f1204423bd9f8a2ab309569903 Mon Sep 17 00:00:00 2001 From: David de Hilster Date: Wed, 3 Jun 2026 13:14:51 -0400 Subject: [PATCH 1/2] Add explicit Engine.close() / context-manager + bump engine to v3.1.55 Companion to nlp-engine v3.1.55 (NLP-ENGINE-523), which made NLP_ENGINE::close() idempotent so it can be called explicitly from Python without breaking the existing ~NLP_ENGINE -> close() path. Changes: 1. bindings.cpp: expose .def("close", &NLP_ENGINE::close, ...) so Python code can deterministically tear down the engine before the auto-created TemporaryDirectory is removed. 2. NLPPlus/__init__.py Engine: - New explicit .close() method that calls self.engine.close() and then self.tmpdir.cleanup() in that order. Idempotent. After close(), further analyze()/compile() calls are undefined behavior. - New __enter__ / __exit__ so `with Engine() as e:` gets deterministic cleanup at the end of the block. - New __del__ that best-effort calls close(). Swallows exceptions because during interpreter shutdown the bindings module may already be torn down. - Initialized self._closed = False guard. - AttributeError fallback in close() in case someone runs the new NLPPlus against an older engine binding that doesn't have .close() exposed -- behaves the same as before in that case. 3. nlp-engine submodule bumped from v3.1.54 to v3.1.55. 4. TemporaryDirectory(ignore_cleanup_errors=True) is kept as defense-in-depth, not the load-bearing fix. Users who use `with Engine() as e:` or call e.close() get a clean rmtree; users who let __del__ tear things down at interpreter shutdown are also fine (best-effort close() runs first, and the ignore-flag catches the residual case where shutdown ordering prevents __del__ from running at all -- crash, os._exit, etc). Test plan: # explicit close e = NLPPlus.Engine() e.analyze("hello world", "parse-en-us") e.close() # tempdir gone, no PermissionError on Windows # context manager with NLPPlus.Engine() as e: e.analyze("hello world", "parse-en-us") # tempdir gone # legacy "just let it GC" (still works, ignore_cleanup_errors=True # catches the residual case if __del__ doesn't run) e = NLPPlus.Engine() e.analyze("hello world", "parse-en-us") del e Co-Authored-By: Claude Opus 4.7 --- NLPPlus/__init__.py | 71 +++++++++++++++++++++++++++++++++++++++------ bindings.cpp | 13 ++++++++- nlp-engine | 2 +- 3 files changed, 75 insertions(+), 11 deletions(-) diff --git a/NLPPlus/__init__.py b/NLPPlus/__init__.py index 270ff88..8778749 100644 --- a/NLPPlus/__init__.py +++ b/NLPPlus/__init__.py @@ -83,16 +83,18 @@ def __init__( verbose: bool = False, initialize: bool = False, ): + self._closed = False if working_folder is None: - # ignore_cleanup_errors=True: the nlp-engine's static `cgerr` - # ofstream holds /logs/cgerr.log open for the - # lifetime of the process. Python's TemporaryDirectory - # cleanup runs during atexit, but the C++ static destructor - # that would close cgerr runs *after* atexit (Python returns - # control to the OS first). On Windows that ordering means - # cleanup hits a still-open file and raises PermissionError. - # Swallow it — the OS reclaims the tempdir at process exit - # regardless. Tracked engine-side as NLP-ENGINE-523. + # ignore_cleanup_errors=True is now defense-in-depth: close() + # / __exit__ / __del__ explicitly call self.engine.close() + # before TemporaryDirectory.cleanup, which closes the engine's + # cgerr.log handle and lets Windows delete the temp dir + # cleanly. The flag still catches the corner case of an + # interpreter shutdown where __del__ never runs (e.g. crash, + # os._exit) — the OS reclaims the tempdir at process exit + # regardless, so swallowing the cleanup error keeps stderr + # quiet for users who never explicitly close. Engine-side + # fix shipped in NLP-ENGINE-523 (engine v3.1.55+). self.tmpdir = TemporaryDirectory( prefix="NLPPlus-", ignore_cleanup_errors=True ) @@ -118,6 +120,57 @@ def __init__( ) self.engine = NLP_ENGINE(str(self.working_folder), silent=not verbose) + def close(self): + """Tear down the underlying engine and release the working folder. + + Idempotent: safe to call multiple times. After ``close()``, any + call to :meth:`analyze`, :meth:`compile`, or :meth:`cloud_compile` + is undefined behavior — create a new ``Engine`` instead. + + On Windows in particular, calling ``close()`` (or using ``Engine`` + as a context manager) is what makes the auto-created + ``TemporaryDirectory`` working folder delete cleanly: the engine + keeps a file handle on ``/logs/cgerr.log`` open for + the lifetime of the C++ instance, and Windows refuses to delete + a directory that contains an open file. ``close()`` calls into + the C++ engine's ``close()`` (NLP-ENGINE-523, engine v3.1.55+), + which releases that handle before the tempdir is removed. + """ + if self._closed: + return + self._closed = True + # Engine.close() is idempotent on the C++ side as of engine + # v3.1.55; older engines just no-op the second teardown. + try: + self.engine.close() + except AttributeError: + # Pre-3.1.55 binding without the close() method exposed; + # fall back to letting __del__ tear it down. The tempdir + # cleanup below will still hit the PermissionError on + # Windows in that case — same situation as before 2.0.4. + pass + if self.tmpdir is not None: + self.tmpdir.cleanup() + self.tmpdir = None + + def __enter__(self): + return self + + def __exit__(self, exc_type, exc_value, traceback): + self.close() + return False + + def __del__(self): + # Best-effort: __del__ may run during interpreter shutdown when + # modules are being torn down and the bindings module may already + # be gone. Swallow anything that goes wrong here; the explicit + # close() / context-manager paths are the supported way to get + # deterministic cleanup. + try: + self.close() + except Exception: + pass + def analyze(self, text: str, analyzer_name: str, develop: bool = False, compiled: bool = False) -> Results: """Analyze text with the named analyzer. diff --git a/bindings.cpp b/bindings.cpp index 0db873f..53e1150 100644 --- a/bindings.cpp +++ b/bindings.cpp @@ -119,5 +119,16 @@ NB_MODULE(bindings, m) { "Generates /run/*.cpp and /kb/*.cpp\n" "(or just /kb/*.cpp if `kbOnly=True`). Those\n" "still need to be built into shared libraries before\n" - "`analyze(..., compiled=True)` can load them."); + "`analyze(..., compiled=True)` can load them.") + .def("close", &NLP_ENGINE::close, + "Tear down the engine's VTRun runtime and release the open\n" + "/logs/cgerr.log handle. Safe to call multiple\n" + "times (engine v3.1.55+ NLP-ENGINE-523 made close()\n" + "idempotent). After close() returns, any subsequent\n" + "analyze()/compile() call on this instance is undefined\n" + "behavior. NLPPlus.Engine.close() / __exit__ / __del__\n" + "calls this before deleting the TemporaryDirectory backing\n" + "the working folder; this is what makes the temp-dir\n" + "cleanup safe on Windows where an open file handle would\n" + "otherwise block the rmtree."); } diff --git a/nlp-engine b/nlp-engine index d95e5e9..6de5176 160000 --- a/nlp-engine +++ b/nlp-engine @@ -1 +1 @@ -Subproject commit d95e5e92d19fbe3e8893088bc0510d8d83231997 +Subproject commit 6de51766c0b877e4a2ecbca7742c2265b35a1abe From d38a624bc07c9fcb3afbe0787bc75e187ce2fc4a Mon Sep 17 00:00:00 2001 From: David de Hilster Date: Wed, 3 Jun 2026 13:19:49 -0400 Subject: [PATCH 2/2] bindings: disambiguate overloaded NLP_ENGINE::close in .def() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit NLP_ENGINE has two close() overloads in include/Api/lite/nlp_engine.h: int close(); int close(_TCHAR *analyzer); So &NLP_ENGINE::close is an unresolved overloaded function type and g++ rejects nb::class_<>::def("close", &NLP_ENGINE::close, ...) with: error: no matching function for call to 'nanobind::class_::def(const char [6], , const char [521])' Cast the member function pointer to the nullary overload — that's the global-teardown one we want (the close(analyzer) overload removes a single named analyzer from VTRun, unrelated to NLP-ENGINE-523). Co-Authored-By: Claude Opus 4.7 --- bindings.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/bindings.cpp b/bindings.cpp index 53e1150..04431b4 100644 --- a/bindings.cpp +++ b/bindings.cpp @@ -120,7 +120,11 @@ NB_MODULE(bindings, m) { "(or just /kb/*.cpp if `kbOnly=True`). Those\n" "still need to be built into shared libraries before\n" "`analyze(..., compiled=True)` can load them.") - .def("close", &NLP_ENGINE::close, + // NLP_ENGINE has two close() overloads: close() and + // close(_TCHAR *analyzer). Cast to pick the nullary one + // (we want the global teardown, not the per-analyzer one). + .def("close", + static_cast(&NLP_ENGINE::close), "Tear down the engine's VTRun runtime and release the open\n" "/logs/cgerr.log handle. Safe to call multiple\n" "times (engine v3.1.55+ NLP-ENGINE-523 made close()\n"