Closing one engine no longer breaks another in the same process (#791) (4.2.35)

.github/workflows/build-windows.yml

@@ -1578,3 +1578,17 @@ jobs:
           mode: addition
           tolerance: same
           output: parse-en-us-diff-win.txt
+
+      - name: Two engines in one process (#791)
+        shell: bash
+        run: |
+          set -eo pipefail
+          # Runs after the analyzers checkout: test-nlp loads parse-en-us from
+          # ./analyzers. It drives NLP_ENGINE through its C++ API: analyzers,
+          # then two engines open at once, closing one and using the other.
+          # Closing an engine used to free the analyzer names of every engine
+          # made before it, which then crashed or produced nothing (#791).
+          cmake --build build --config Release --target test-nlp
+          ./bin/Release/test-nlp.exe > test-nlp.out 2>&1 || { tail -40 test-nlp.out; exit 1; }
+          grep -q "^ok" test-nlp.out || { tail -40 test-nlp.out; exit 1; }
+          echo "[two-engines] closing one engine leaves the other working"

include/Api/lite/vtrun.h

@@ -56,6 +56,9 @@ class LITE_API VTRun
 	void zero();                  // Zero out ds.
 
 	void *getHtab();	// 06/24/03 AM.
+	// Make this runtime's string table the process default (Htab::gstab_),
+	// which analyzer loading interns rule and function names into (#791).
+	void useStab();
 
    bool isBuiltin(_TCHAR *name);
 

lite/nlp_engine.cpp

@@ -210,6 +210,10 @@ int NLP_ENGINE::init(
     // If calling a different analyzer, clear out info. // [DEGLOB]	// 10/15/20 AM.
     // Todo: The NLP object should store all analyzer information.  // [DEGLOB]	// 10/15/20 AM.
     NLP_ENGINE::zeroAna();  // [DEGLOB]	// 10/15/20 AM.
+    // This engine's string table must be the process default while it
+    // loads analyzers, or their names land in another engine's (#791).
+    if (m_vtrun)
+        m_vtrun->useStab();
 
     // Convenience ptr.
     //m_vtrun = VTRun_Ptr;    // [DEGLOB]	// 10/15/20 AM.
@@ -432,6 +436,8 @@ int NLP_ENGINE::analyze(
     bool compileAna
 	)
 {
+    if (m_vtrun)
+        m_vtrun->useStab();     // #791.
     // Respect init() failure. On a re-entrant analyze() in a long-lived process
     // (e.g. the node/python addon), a name-collision in addAna can leave a fresh
     // NLP unregistered; if make_analyzer/makeCG then fails, init() deletes m_nlp
@@ -501,6 +507,8 @@ int NLP_ENGINE::analyzeStream(
     bool compiled
 	)
 {
+    if (m_vtrun)
+        m_vtrun->useStab();     // #791.
     if (NLP_ENGINE::init(analyzer,develop,silent,false,compiled,false,false) < 0)
         return -1;
 
@@ -648,6 +656,8 @@ int NLP_ENGINE::analyze(
     bool compileAna
 	)
 {
+    if (m_vtrun)
+        m_vtrun->useStab();     // #791.
 
     // Respect init() failure (see file-I/O overload above): bail instead of
     // dereferencing a freed m_nlp.
@@ -697,6 +707,8 @@ int NLP_ENGINE::analyze(
     bool compileAna
 	)
 {
+    if (m_vtrun)
+        m_vtrun->useStab();     // #791.
 
     // Respect init() failure (see file-I/O overload above): bail instead of
     // dereferencing a freed m_nlp.

lite/vtrun.cpp

@@ -122,6 +122,23 @@ void *VTRun::getHtab()
 return htab_;
 }
 
+/********************************************
+* FN:		USESTAB
+* SUBJ:	Make this runtime's string table the process default (#791).
+* NOTE:	Htab::gstab_ is one per process, and analyzer loading interns rule
+*			and function names into it. Each runtime used to set it only when
+*			made, so with two engines open, the one made last owned every
+*			analyzer's names; closing it freed them under the other engine,
+*			whose next analyzer load crashed or produced nothing. Each
+*			NLP_ENGINE entry point now calls this first.
+********************************************/
+
+void VTRun::useStab()
+{
+if (stab_)
+	Htab::setGstab((Stab *)stab_);
+}
+
 
 //*******************************************
 
@@ -159,7 +176,12 @@ if (htab_)
 	delete (Htab *) htab_;
 	}
 if (stab_)
+	{
+	// Don't leave the process default pointing at a freed table (#791).
+	if (Htab::getGstab() == (Stab *) stab_)
+		Htab::setGstab(0);
 	delete (Stab *) stab_;
+	}
 if (htfunc_)
 	Ifunc::deleteHtab(htfunc_);
 if (alist_)

nlp/main.cpp

@@ -15,7 +15,7 @@ All rights reserved.
 #include "lite/nlp_engine.h"
 #include "version.h"
 
-#define NLP_ENGINE_VERSION "4.2.34"
+#define NLP_ENGINE_VERSION "4.2.35"
 
 bool cmdReadArgs(int, _TCHAR *argv[], _TCHAR *&, _TCHAR *&, _TCHAR *&, _TCHAR *&, bool &, bool &, bool &, bool &, bool &, bool &, int &, bool &, bool &);
 void cmdHelpargs(_TCHAR *);

test/main.cpp

@@ -110,6 +110,35 @@ int _tmain( // 09/23/20 AM.
         ++failures;
     }
 
+    // Two engines open in one process (#791): closing one must leave the
+    // other working. Each engine set the process-wide string table only when
+    // made, so an analyzer loaded by the first engine after a second was made
+    // kept its names in the second's table, and closing the second left the
+    // first crashing or producing nothing.
+    NLP_ENGINE *first = new NLP_ENGINE(workingFolder);
+    for (int cycle = 0; cycle < 3; ++cycle) {
+        NLP_ENGINE *second = new NLP_ENGINE(workingFolder);
+        std::istringstream ssi4("A second engine.");
+        std::ostringstream sso4;
+        second->analyze(anaParse,&ssi4,&sso4);
+        std::istringstream ssi5("Hello world!");
+        std::ostringstream sso5;
+        first->analyze(anaParse,&ssi5,&sso5);		// Loads while second is newest.
+        second->close();
+        delete second;
+        std::istringstream ssi6("Hello again.");
+        std::ostringstream sso6;
+        if (first->analyze(anaParse,&ssi6,&sso6) < 0 || sso6.str().empty()) {
+            std::_t_cout << _T("[FAIL: an engine broken after closing another (#791), cycle ")
+                         << cycle << _T("]") << std::endl;
+            ++failures;
+            break;
+        }
+    }
+    first->close();
+    delete first;
+    std::_t_cout << _T("[two engines: ok unless FAIL above]") << std::endl;
+
     delete nlpEngine;
     std::_t_cout << (failures ? _T("FAILED") : _T("ok")) << std::endl;
     return failures ? 1 : 0;