From e12de7b48dfe92bef1d9c58eaf93222536736a53 Mon Sep 17 00:00:00 2001
From: bneradt <bneradt@verizonmedia.com>
Date: Mon, 25 Jan 2021 18:08:57 +0000
Subject: [PATCH] Move reopen_moved_log_files to log flushing thread

The handling of the log rotation signal was locking against the periodic
flushing thread. This moves the log rotation handling logic into the
same flushing thread so they will not lock against each other.
---
 proxy/logging/Log.cc                     | 19 +++++++++++++------
 proxy/logging/Log.h                      |  8 +++++---
 src/traffic_server/traffic_server.cc     |  2 +-
 tests/gold_tests/logging/sigusr2.test.py |  1 +
 4 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/proxy/logging/Log.cc b/proxy/logging/Log.cc
index 61307e0abc2..0766256167e 100644
--- a/proxy/logging/Log.cc
+++ b/proxy/logging/Log.cc
@@ -72,6 +72,7 @@ int Log::preproc_threads;
 int Log::init_status                  = 0;
 int Log::config_flags                 = 0;
 bool Log::logging_mode_changed        = false;
+bool Log::log_rotate_signal_received  = false;
 uint32_t Log::periodic_tasks_interval = PERIODIC_TASKS_INTERVAL_FALLBACK;
 
 // Hash table for LogField symbols
@@ -140,12 +141,6 @@ Log::change_configuration()
   Debug("log-config", "... new configuration in place");
 }
 
-void
-Log::reopen_moved_log_files()
-{
-  Log::config->log_object_manager.reopen_moved_log_files();
-}
-
 /*-------------------------------------------------------------------------
   PERIODIC EVENTS
 
@@ -260,6 +255,10 @@ Log::periodic_tasks(long time_now)
       }
       Log::config->log_object_manager.roll_files(time_now);
     }
+    if (log_rotate_signal_received) {
+      Log::config->log_object_manager.reopen_moved_log_files();
+      log_rotate_signal_received = false;
+    }
   }
 }
 
@@ -986,6 +985,14 @@ Log::handle_periodic_tasks_int_change(const char * /* name ATS_UNUSED */, RecDat
   return REC_ERR_OKAY;
 }
 
+int
+Log::handle_log_rotation_request()
+{
+  Debug("log", "Request to reopen rotated log files.");
+  log_rotate_signal_received = true;
+  return 0;
+}
+
 void
 Log::init(int flags)
 {
diff --git a/proxy/logging/Log.h b/proxy/logging/Log.h
index eff338aca7f..50c4323ff4b 100644
--- a/proxy/logging/Log.h
+++ b/proxy/logging/Log.h
@@ -206,15 +206,16 @@ class Log
   // reconfiguration stuff
   static void change_configuration();
 
+  static int handle_logging_mode_change(const char *name, RecDataT data_type, RecData data, void *cookie);
+  static int handle_periodic_tasks_int_change(const char *name, RecDataT data_type, RecData data, void *cookie);
+
   /** Check each log file path to see whether it exists and re-open if not.
    *
    * This is called when an external log rotation entity has moved log files to
    * rolled names. This checks whether the original log file exists and, if
    * not, closes the file descriptor and re-opens the file.
    */
-  static void reopen_moved_log_files();
-  static int handle_logging_mode_change(const char *name, RecDataT data_type, RecData data, void *cookie);
-  static int handle_periodic_tasks_int_change(const char *name, RecDataT data_type, RecData data, void *cookie);
+  static int handle_log_rotation_request();
 
   friend void RegressionTest_LogObjectManager_Transfer(RegressionTest *, int, int *);
 
@@ -226,6 +227,7 @@ class Log
   static int init_status;
   static int config_flags;
   static bool logging_mode_changed;
+  static bool log_rotate_signal_received;
   static uint32_t periodic_tasks_interval;
 };
 
diff --git a/src/traffic_server/traffic_server.cc b/src/traffic_server/traffic_server.cc
index 4f54e9846ce..faf317d8fa6 100644
--- a/src/traffic_server/traffic_server.cc
+++ b/src/traffic_server/traffic_server.cc
@@ -290,7 +290,7 @@ class SignalContinuation : public Continuation
         Note("Could not reseat %s", DIAGS_LOG_FILENAME);
       }
       // Reload any of the other moved log files (such as the ones in logging.yaml).
-      Log::reopen_moved_log_files();
+      Log::handle_log_rotation_request();
     }
 
     if (signal_received[SIGTERM] || signal_received[SIGINT]) {
diff --git a/tests/gold_tests/logging/sigusr2.test.py b/tests/gold_tests/logging/sigusr2.test.py
index a1d2ed83bdd..cc01166e83b 100644
--- a/tests/gold_tests/logging/sigusr2.test.py
+++ b/tests/gold_tests/logging/sigusr2.test.py
@@ -43,6 +43,7 @@ def __configure_traffic_manager(self):
             'proxy.config.http.wait_for_cache': 1,
             'proxy.config.diags.debug.enabled': 1,
             'proxy.config.diags.debug.tags': 'log',
+            'proxy.config.log.periodic_tasks_interval': 1,
 
             # All log rotation should be handled externally.
             'proxy.config.log.rolling_enabled': 0,