(trafficserver) branch master updated: Fix lock-order inversion deadlock in Diags::tag_activated (#13106)

[bneradt]

This is an automated email from the ASF dual-hosted git repository.

bneradt pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficserver.git


The following commit(s) were added to refs/heads/master by this push:
     new 571ee72309 Fix lock-order inversion deadlock in Diags::tag_activated 
(#13106)
571ee72309 is described below

commit 571ee72309a6935e83b017e5618ddb788af019df
Author: Brian Neradt <brian.ner...@gmail.com>
AuthorDate: Thu Apr 23 18:06:14 2026 -0500

    Fix lock-order inversion deadlock in Diags::tag_activated (#13106)
---
 include/tscore/DiagsTypes.h |  7 ++++---
 src/tscore/Diags.cc         | 42 ++++++++++++++++++++++++------------------
 2 files changed, 28 insertions(+), 21 deletions(-)

diff --git a/include/tscore/DiagsTypes.h b/include/tscore/DiagsTypes.h
index f770e871c4..1b3713160b 100644
--- a/include/tscore/DiagsTypes.h
+++ b/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 @@ public:
   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;
diff --git a/src/tscore/Diags.cc b/src/tscore/Diags.cc
index 14a1b3436e..6fe5b79cf8 100644
--- a/src/tscore/Diags.cc
+++ b/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); });