HPCC-23130 Re-enable threadLocal option for embedded Java

Previous fix (in 7.6.14-rc1) caused stackfaults if reusing threads via
threadpools.

Unregister threadhooks once called, where posible, to avoid them being called
after dlls are unloaded (or at least reduce the likelihood).

Signed-off-by: Richard Chapman <rchapman@hpccsystems.com>

plugins/javaembed/javaembed.cpp

@@ -4646,9 +4646,8 @@ protected:
 };
 
 static __thread JavaThreadContext* threadContext;  // We reuse per thread, for speed
-static __thread ThreadTermFunc threadHookChain;
 
-static void releaseContext(bool isPooled)
+static bool releaseContext(bool isPooled)
 {
     if (threadContext)
     {
@@ -4658,11 +4657,10 @@ static void releaseContext(bool isPooled)
             delete threadContext;
             threadContext = NULL;
         }
+        else
+            return true;
     }
-    if (threadHookChain)
-    {
-        (*threadHookChain)(isPooled);
-    }
+    return false;
 }
 
 static JavaThreadContext *queryContext()
@@ -4670,7 +4668,7 @@ static JavaThreadContext *queryContext()
     if (!threadContext)
     {
         threadContext = new JavaThreadContext;
-        threadHookChain = addThreadTermFunc(releaseContext);
+        addThreadTermFunc(releaseContext);
     }
     return threadContext;
 }

plugins/memcached/memcachedplugin.cpp

@@ -125,7 +125,6 @@ typedef Owned<MCached> OwnedMCached;
 static const unsigned MAX_TYPEMISMATCHCOUNT = 10;
 
 static __thread MCached * cachedConnection;
-static __thread ThreadTermFunc threadHookChain;
 static __thread bool threadHooked;
 
 //The following class is here to ensure destruction of the cachedConnection within the main thread
@@ -144,17 +143,15 @@ public :
     }
 } mainThread;
 
-static void releaseContext(bool isPooled)
+static bool releaseContext(bool isPooled)
 {
     if (cachedConnection)
     {
         cachedConnection->Release();
         cachedConnection = NULL;
     }
-    if (threadHookChain)
-    {
-        (*threadHookChain)(isPooled);
-    }
+    threadHooked = false;
+    return false;
 }
 MCached * createConnection(ICodeContext * ctx, const char * options)
 {
@@ -163,7 +160,7 @@ MCached * createConnection(ICodeContext * ctx, const char * options)
         cachedConnection = new MemCachedPlugin::MCached(ctx, options);
         if (!threadHooked)
         {
-            threadHookChain = addThreadTermFunc(releaseContext);
+            addThreadTermFunc(releaseContext);
             threadHooked = true;
         }
         return LINK(cachedConnection);

plugins/mysql/mysqlembed.cpp

@@ -1372,21 +1372,16 @@ protected:
 
 // Each call to a MySQL function will use a new MySQLEmbedFunctionContext object
 
-static __thread ThreadTermFunc threadHookChain;
-
 static bool mysqlInitialized = false;
 static __thread bool mysqlThreadInitialized = false;
 static CriticalSection initCrit;
 
-static void terminateMySqlThread(bool isPooled)
+static bool terminateMySqlThread(bool isPooled)
 {
     MySQLConnection::clearThreadCache();
     mysql_thread_end();
     mysqlThreadInitialized = false;  // In case it was a threadpool thread...
-    if (threadHookChain)
-    {
-        (*threadHookChain)(isPooled);
-    }
+    return false;
 }
 
 static void initializeMySqlThread()
@@ -1402,7 +1397,7 @@ static void initializeMySqlThread()
             }
         }
         mysql_thread_init();
-        threadHookChain = addThreadTermFunc(terminateMySqlThread);
+        addThreadTermFunc(terminateMySqlThread);
         mysqlThreadInitialized = true;
     }
 }

plugins/py3embed/py3embed.cpp

@@ -268,19 +268,15 @@ private:
 };
 
 static __thread PythonThreadContext* threadContext;  // We reuse per thread, for speed
-static __thread ThreadTermFunc threadHookChain;
 
-static void releaseContext(bool isPooled)
+static bool releaseContext(bool isPooled)
 {
     if (threadContext)
     {
         delete threadContext;
         threadContext = NULL;
     }
-    if (threadHookChain)
-    {
-        (*threadHookChain)(isPooled);
-    }
+    return false;
 }
 
 // Use a global object to ensure that the Python interpreter is initialized on main thread
@@ -612,7 +608,7 @@ static void checkThreadContext()
         if (!globalState.isInitialized())
             rtlFail(0, "Python not initialized");
         threadContext = new PythonThreadContext;
-        threadHookChain = addThreadTermFunc(releaseContext);
+        addThreadTermFunc(releaseContext);
     }
 }
 

plugins/pyembed/pyembed.cpp

@@ -262,19 +262,15 @@ private:
 };
 
 static __thread PythonThreadContext* threadContext;  // We reuse per thread, for speed
-static __thread ThreadTermFunc threadHookChain;
 
-static void releaseContext(bool isPooled)
+static bool releaseContext(bool isPooled)
 {
     if (threadContext)
     {
         delete threadContext;
         threadContext = NULL;
     }
-    if (threadHookChain)
-    {
-        (*threadHookChain)(isPooled);
-    }
+    return false;
 }
 
 // Use a global object to ensure that the Python interpreter is initialized on main thread
@@ -609,7 +605,7 @@ static void checkThreadContext()
         if (!globalState.isInitialized())
             rtlFail(0, "Python not initialized");
         threadContext = new PythonThreadContext;
-        threadHookChain = addThreadTermFunc(releaseContext);
+        addThreadTermFunc(releaseContext);
     }
 }
 

plugins/redis/redis.cpp

@@ -58,7 +58,6 @@ static __thread Connection * cachedSubscriptionConnection = nullptr;
 #define DUMMY_PORT 0
 
 static CriticalSection critsec;
-static __thread ThreadTermFunc threadHookChain = nullptr;
 static __thread bool threadHooked = false;
 static int connectionCachingLevel = ALLOW_CONNECTION_CACHING;
 static std::atomic<bool> connectionCachingLevelChecked(false);
@@ -258,7 +257,7 @@ public :
 
     Owned<Connection> connection;
 };
-static void releaseAllCachedContexts(bool isPooled)
+static bool releaseAllCachedContexts(bool isPooled)
 {
     if (cachedConnection)
     {
@@ -275,10 +274,8 @@ static void releaseAllCachedContexts(bool isPooled)
         cachedSubscriptionConnection->Release();
         cachedSubscriptionConnection = nullptr;
     }
-    if (threadHookChain)
-    {
-        (*threadHookChain)(isPooled);
-    }
+    threadHooked = false;
+    return false;
 }
 //The following class is here to ensure destruction of the cachedConnection within the main thread
 //as this is not handled by the thread hook mechanism.
@@ -762,7 +759,7 @@ static void addThreadHook()
 {
     if (!threadHooked)
     {
-        threadHookChain = addThreadTermFunc(releaseAllCachedContexts);
+        addThreadTermFunc(releaseAllCachedContexts);
         threadHooked = true;
     }
 }

plugins/v8embed/v8embed.cpp

@@ -936,19 +936,15 @@ protected:
 };
 
 static __thread V8JavascriptEmbedFunctionContext * theFunctionContext;  // We reuse per thread, for speed
-static __thread ThreadTermFunc threadHookChain;
 
-static void releaseContext(bool isPooled)
+static bool releaseContext(bool isPooled)
 {
     if (theFunctionContext)
     {
         ::Release(theFunctionContext);
         theFunctionContext = NULL;
     }
-    if (threadHookChain)
-    {
-        (*threadHookChain)(isPooled);
-    }
+    return false;
 }
 
 class V8JavascriptEmbedContext : public CInterfaceOf<IEmbedContext>
@@ -968,7 +964,7 @@ public:
         if (!theFunctionContext)
         {
             theFunctionContext = new V8JavascriptEmbedFunctionContext;
-            threadHookChain = addThreadTermFunc(releaseContext);
+            addThreadTermFunc(releaseContext);
         }
         theFunctionContext->setActivityContext(activityContext);
         return LINK(theFunctionContext);

rtl/eclrtl/eclrtl.cpp

@@ -333,17 +333,14 @@ private:
 
 typedef MapStringTo<RTLUnicodeConverter, char const *> MapStrToUnicodeConverter;
 static __thread MapStrToUnicodeConverter *unicodeConverterMap = NULL;
-static __thread ThreadTermFunc prevThreadTerminator = NULL;
 static __thread bool threadHooked = false;
 
-static void clearUnicodeConverterMap(bool isPooled)
+static bool clearUnicodeConverterMap(bool isPooled)
 {
     delete unicodeConverterMap;
     unicodeConverterMap = NULL;  // Important to clear, as this is called when threadpool threads end...
-    if (prevThreadTerminator)
-    {
-        (*prevThreadTerminator)(isPooled);
-    }
+    threadHooked = false;
+    return false;
 }
 
 RTLUnicodeConverter * queryRTLUnicodeConverter(char const * codepage)
@@ -355,7 +352,7 @@ RTLUnicodeConverter * queryRTLUnicodeConverter(char const * codepage)
         // NB: May need to revisit if not on a jlib Thread.
         if (!threadHooked)
         {
-            prevThreadTerminator = addThreadTermFunc(clearUnicodeConverterMap);
+            addThreadTermFunc(clearUnicodeConverterMap);
             threadHooked = true;
         }
     }

system/jlib/jthread.cpp

@@ -39,21 +39,28 @@
 
 //#define NO_CATCHALL
 
-static __thread ThreadTermFunc threadTerminationHook;
+//static __thread ThreadTermFunc threadTerminationHook;
+static thread_local std::vector<ThreadTermFunc> threadTermHooks;
 
-ThreadTermFunc addThreadTermFunc(ThreadTermFunc onTerm)
+void addThreadTermFunc(ThreadTermFunc onTerm)
 {
-    ThreadTermFunc old = threadTerminationHook;
-    threadTerminationHook = onTerm;
-    return old;
+    for (auto hook: threadTermHooks)
+    {
+        if (hook==onTerm)
+            return;
+    }
+    threadTermHooks.push_back(onTerm);
 }
 
 void callThreadTerminationHooks(bool isPooled)
 {
-    if (threadTerminationHook)
+    std::vector<ThreadTermFunc> keepHooks;
+    for (auto hook: threadTermHooks)
     {
-        (*threadTerminationHook)(isPooled);
+        if ((*hook)(isPooled) && isPooled)
+           keepHooks.push_back(hook);
     }
+    threadTermHooks.swap(keepHooks);
 }
 
 PointerArray *exceptionHandlers = NULL;
@@ -64,8 +71,10 @@ MODULE_INIT(INIT_PRIORITY_JTHREAD)
 }
 MODULE_EXIT()
 {
-    if (threadTerminationHook)
-        (*threadTerminationHook)(false);  // May be too late :(
+    for (auto hook: threadTermHooks)
+    {
+        (*hook)(false);  // May be too late :(
+    }
     delete exceptionHandlers;
 }
 

system/jlib/jthread.hpp

@@ -60,8 +60,8 @@ extern jlib_decl unsigned threadLogID();  // for use in logging
 // so the hook function should clear any variables if necessary rather than assuming that they will be cleared
 // at thread startup time.
 
-typedef void (*ThreadTermFunc)(bool isPooled);
-extern jlib_decl ThreadTermFunc addThreadTermFunc(ThreadTermFunc onTerm);
+typedef bool (*ThreadTermFunc)(bool isPooled);
+extern jlib_decl void addThreadTermFunc(ThreadTermFunc onTerm);
 extern jlib_decl void callThreadTerminationHooks(bool isPooled);
 
 //An exception safe way of ensuring that the thread termination hooks are called.