diff --git a/.github/workflows/build-windows.yml b/.github/workflows/build-windows.yml index 9b645bfc..02e58ddd 100644 --- a/.github/workflows/build-windows.yml +++ b/.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" diff --git a/include/Api/lite/vtrun.h b/include/Api/lite/vtrun.h index 709d2e72..00ec680c 100644 --- a/include/Api/lite/vtrun.h +++ b/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); diff --git a/lite/nlp_engine.cpp b/lite/nlp_engine.cpp index 8a2fff05..4ad7029b 100644 --- a/lite/nlp_engine.cpp +++ b/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. diff --git a/lite/vtrun.cpp b/lite/vtrun.cpp index 7687e07d..6e3f52b6 100644 --- a/lite/vtrun.cpp +++ b/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_) diff --git a/nlp/main.cpp b/nlp/main.cpp index 2cda5df3..0954ac2b 100644 --- a/nlp/main.cpp +++ b/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 *); diff --git a/test/main.cpp b/test/main.cpp index cbe4c288..6ccff229 100644 --- a/test/main.cpp +++ b/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;