Fix lock-order inversion deadlock in Diags::tag_activated

include/tscore/DiagsTypes.h

@@ -32,6 +32,7 @@
 #pragma once
 
 #include <cstdarg>
+#include <memory>
 #include <string>
 #include <string_view>
 #include "tsutil/DbgCtl.h"
@@ -242,9 +243,9 @@ class Diags : public DebugInterface
   IpAddr debug_client_ip;
 
 private:
-  const std::string prefix_str;
-  mutable ink_mutex tag_table_lock;    // prevents reconfig/read races
-  Regex            *activated_tags[2]; // 1 table for debug, 1 for action
+  const std::string      prefix_str;
+  mutable ink_mutex      tag_table_lock;    // prevents reconfig/read races
+  std::shared_ptr<Regex> activated_tags[2]; // 1 table for debug, 1 for action
 
   // These are the default logfile permissions
   int diags_logfile_perm  = -1;

src/tscore/Diags.cc

@@ -328,19 +328,26 @@ Diags::print_va(const char *debug_tag, DiagsLevel diags_level, const SourceLocat
 bool
 Diags::tag_activated(const char *tag, DiagsTagType mode) const
 {
-  bool activated = false;
-
   if (tag == nullptr) {
-    return (true);
+    return true;
   }
 
-  lock();
-  if (activated_tags[mode]) {
-    activated = activated_tags[mode]->exec(tag, RE_ANCHORED);
+  // Snapshot the regex under the lock, then release the lock before running
+  // Regex::exec().  exec() can lazily construct a thread_local RegexContext
+  // whose destructor registration via __cxa_thread_atexit_impl takes the
+  // dynamic loader lock.  Holding tag_table_lock across exec() therefore
+  // creates a lock-order inversion with dlopen() callers that construct a
+  // DbgCtl during a plugin's static initialization (which takes
+  // tag_table_lock while already holding the dl loader lock).  See the
+  // analogous fix in DbgCtl::_new_reference.
+  std::shared_ptr<Regex> regex;
+  {
+    lock();
+    regex = activated_tags[mode];
+    unlock();
   }
-  unlock();
 
-  return (activated);
+  return regex ? regex->exec(tag, RE_ANCHORED) : false;
 }
 
 //////////////////////////////////////////////////////////////////////////////
@@ -358,13 +365,15 @@ void
 Diags::activate_taglist(const char *taglist, DiagsTagType mode)
 {
   if (taglist) {
-    lock();
-    if (activated_tags[mode]) {
-      delete activated_tags[mode];
+    // Compile the new regex outside tag_table_lock to avoid holding the lock
+    // across work that may touch thread_local state (see tag_activated).
+    auto new_regex = std::make_shared<Regex>();
+    new_regex->compile(taglist);
+    {
+      lock();
+      activated_tags[mode] = std::move(new_regex);
+      unlock();
     }
-    activated_tags[mode] = new Regex;
-    activated_tags[mode]->compile(taglist);
-    unlock();
   }
   if ((DiagsTagType_Debug == mode) && (this == diags())) {
     DbgCtl::update([&](const char *tag) -> bool { return tag_activated(tag, DiagsTagType_Debug); });
@@ -385,10 +394,7 @@ void
 Diags::deactivate_all(DiagsTagType mode)
 {
   lock();
-  if (activated_tags[mode]) {
-    delete activated_tags[mode];
-    activated_tags[mode] = nullptr;
-  }
+  activated_tags[mode].reset();
   unlock();
   if ((DiagsTagType_Debug == mode) && (this == diags())) {
     DbgCtl::update([&](const char *tag) -> bool { return tag_activated(tag, DiagsTagType_Debug); });