[trafficserver] branch 9.0.x updated: Issue 4635: Address pipe reuse after configuration reload issues

[zw...@apache.org]

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

zwoop pushed a commit to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git


The following commit(s) were added to refs/heads/9.0.x by this push:
     new 1305e70  Issue 4635: Address pipe reuse after configuration reload issues
1305e70 is described below

commit 1305e7000a50bb16dfa4d047636c2a0a460de09f
Author: bneradt <bn...@verizonmedia.com>
AuthorDate: Fri Oct 25 22:49:31 2019 +0000

    Issue 4635: Address pipe reuse after configuration reload issues
    
    (cherry picked from commit 69a318f2cbb332edcdcfb6a58aec32b91e05f867)
---
 proxy/logging/LogFile.cc   |  6 ++++++
 proxy/logging/LogObject.cc | 19 +++++++------------
 2 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/proxy/logging/LogFile.cc b/proxy/logging/LogFile.cc
index 1d7d0a1..d0b95da 100644
--- a/proxy/logging/LogFile.cc
+++ b/proxy/logging/LogFile.cc
@@ -118,6 +118,12 @@ LogFile::LogFile(const LogFile &copy)
 LogFile::~LogFile()
 {
   Debug("log-file", "entering LogFile destructor, this=%p", this);
+
+  // close_file() checks whether a file is open before attempting to close, so
+  // this is safe to call even if a file had not been opened. Further, calling
+  // close_file() here ensures that we do not leak file descriptors.
+  close_file();
+
   delete m_log;
   ats_free(m_header);
   ats_free(m_name);
diff --git a/proxy/logging/LogObject.cc b/proxy/logging/LogObject.cc
index f1c2337..040c73d 100644
--- a/proxy/logging/LogObject.cc
+++ b/proxy/logging/LogObject.cc
@@ -961,24 +961,20 @@ LogObjectManager::_solve_filename_conflicts(LogObject *log_object, int maxConfli
         LogUtils::manager_alarm(LogUtils::LOG_ALARM_ERROR, msg, filename);
         retVal = CANNOT_SOLVE_FILENAME_CONFLICTS;
       } else {
-        // either the meta file could not be read, or the new object's
-        // signature and the metafile signature do not match ==>
-        // roll old filename so the new object can use the filename
+        // Either the meta file could not be read, or the new object's
+        // signature and the metafile signature do not match.
+        // Roll the old filename so the new object can use the filename
         // it requested (previously we used to rename the NEW file
-        // but now we roll the OLD file), or if the log object writes
-        // to a pipe, just remove the file if it was open as a pipe
+        // but now we roll the OLD file). However, if the log object writes to
+        // a pipe don't roll because rolling is not applicable to pipes.
 
         bool roll_file = true;
 
         if (log_object->writes_to_pipe()) {
-          // determine if existing file is a pipe, and remove it if
-          // that is the case so the right metadata for the new pipe
-          // is created later
-          //
+          // Verify whether the existing file is a pipe. If it is,
+          // disable the roll_file flag so we don't attempt rolling.
           struct stat s;
           if (stat(filename, &s) < 0) {
-            // an error happened while trying to get file info
-            //
             const char *msg = "Cannot stat log file %s: %s";
             char *se        = strerror(errno);
 
@@ -988,7 +984,6 @@ LogObjectManager::_solve_filename_conflicts(LogObject *log_object, int maxConfli
             roll_file = false;
           } else {
             if (S_ISFIFO(s.st_mode)) {
-              unlink(filename);
               roll_file = false;
             }
           }