From bb95b4d75afb677d6d0797a33620386fd197c58d Mon Sep 17 00:00:00 2001
From: Walt Karas <wkaras@yahooinc.com>
Date: Fri, 4 Aug 2023 00:41:01 +0000
Subject: [PATCH] Eliminate function-scope thread_local in Regex.cc.

It appears to be the case that, in the C/C++ runtime for Linux,
there is a mutex that is locked both:
1)  When initializing non-local variables in a shared library,
while it is being loaded by calling dlopen(), and
2)  When execution encounters the definition of a thread_local
variable within a function.  It seems that the mutex is locked
even if the variable has already been initialized, presumably to
check whether it was initialized.

This has been causing problems for the DbgCtl class.  When an
instance of DbgCtl is initialized, it locks a mutex (for the
registry of instances).  The instance initialization then
(indirectly) calls a function (in Regex.cc) that defines a
thread_local varaiable.  The result is scenarios that sometimes
lead to deadlock.  For example:
1)  dlopen() is called to load a plugin.  The runtime's mutex
is taken.
2)  Another thread starts initializing a DbgCtl instance, and
takes the instance registry mutex.
3)  The non-local initialization of the shared library starts
the intialization of another DbgCtl instance.  This thread now
blocks trying to lock the registry mutex.
4)  The other thread now enters the function in Regex.cc that
defines a thread_local object.  It blocks on the runtime's
mutex, which it needs to lock in order to do/check the
thread_local instances.  Deadlock.

This PR fixes the problem, by eliminating the thread_local
variable that's defined in a function.
---
 src/tscore/Regex.cc | 33 +++++++++++++++++++++------------
 1 file changed, 21 insertions(+), 12 deletions(-)

diff --git a/src/tscore/Regex.cc b/src/tscore/Regex.cc
index 22bb2e1c9ec..dc03a9d778d 100644
--- a/src/tscore/Regex.cc
+++ b/src/tscore/Regex.cc
@@ -29,22 +29,31 @@
 #include "tscore/Regex.h"
 
 #ifdef PCRE_CONFIG_JIT
-static pcre_jit_stack *
-get_jit_stack(void *data ATS_UNUSED)
+namespace
 {
-  thread_local struct JitStack {
-    JitStack()
-    {
-      jit_stack = pcre_jit_stack_alloc(ats_pagesize(), 1024 * 1024); // 1 page min and 1MB max
-    }
-    ~JitStack() { pcre_jit_stack_free(jit_stack); }
+thread_local pcre_jit_stack *jit_stack;
 
-    pcre_jit_stack *jit_stack = nullptr;
-  } stack;
+struct JitStackCleanup {
+  ~JitStackCleanup()
+  {
+    if (jit_stack) {
+      pcre_jit_stack_free(jit_stack);
+    }
+  }
+};
+thread_local JitStackCleanup jsc;
 
-  return stack.jit_stack;
+pcre_jit_stack *
+get_jit_stack(void *)
+{
+  if (!jit_stack) {
+    jit_stack = pcre_jit_stack_alloc(ats_pagesize(), 1024 * 1024); // 1 page min and 1MB max
+  }
+  return jit_stack;
 }
-#endif
+
+} // end anonymous namespace
+#endif // def PCRE_CONFIG_JIT
 
 Regex::Regex(Regex &&that) noexcept : regex(that.regex), regex_extra(that.regex_extra)
 {