(trafficserver) branch master updated: Fix potential mutex destroy while locked in origin_server_auth plugin (#12307)

[cmcfarlen]

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

cmcfarlen 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 6e812e3ff7 Fix potential mutex destroy while locked in 
origin_server_auth plugin (#12307)
6e812e3ff7 is described below

commit 6e812e3ff7a03f9ccd2034ad811999bfb4798788
Author: Chris McFarlen <ch...@mcfarlen.us>
AuthorDate: Wed Jul 2 13:26:16 2025 -0500

    Fix potential mutex destroy while locked in origin_server_auth plugin 
(#12307)
    
    * Fix potential mutex destory while locked in origin_server_auth plugin
    
    * reorg code and add explanation
---
 plugins/origin_server_auth/origin_server_auth.cc | 40 +++++++++++++++++++++---
 1 file changed, 35 insertions(+), 5 deletions(-)

diff --git a/plugins/origin_server_auth/origin_server_auth.cc 
b/plugins/origin_server_auth/origin_server_auth.cc
index 6036cca1a5..12c98ce9dd 100644
--- a/plugins/origin_server_auth/origin_server_auth.cc
+++ b/plugins/origin_server_auth/origin_server_auth.cc
@@ -48,6 +48,7 @@
 #include <ts/remap.h>
 #include <ts/remap_version.h>
 #include <tsutil/TsSharedMutex.h>
+#include <utility>
 #include "ts/apidefs.h"
 #include "tscore/ink_config.h"
 #include "swoc/TextView.h"
@@ -224,6 +225,16 @@ public:
     }
   }
 
+  // This handles the hoops needed to safely delete the special
+  // plugin level instance of this class.
+  static void deletePluginInstance(S3Config *p);
+
+  TSCont
+  releaseReloadCont()
+  {
+    return std::exchange(_conf_rld, nullptr);
+  }
+
   // Is this configuration usable?
   bool
   valid() const
@@ -647,6 +658,29 @@ S3Config::parse_config(const std::string &config_fname)
   return true;
 }
 
+void
+S3Config::deletePluginInstance(S3Config *s3)
+{
+  // Due to a race conditions with the background continuation
+  // the plugin shutdown has to take care not to run
+  // when the reload continuation does.
+  //
+  // The order of oprations here is important, be careful making changes
+  if (s3) {
+    TSMutex s3_mutex_ptr = s3->config_reloader_mutex();
+
+    TSMutexLock(s3_mutex_ptr);
+    auto cont = s3->releaseReloadCont();
+    delete s3;
+    TSMutexUnlock(s3_mutex_ptr);
+    // Its important to only destory this continuation outside of holding
+    // its lock, but after it has been cancelled (in the S3Config destructor)
+    if (cont != nullptr) {
+      TSContDestroy(cont);
+    }
+  }
+}
+
 ///////////////////////////////////////////////////////////////////////////////
 // Implementation for the ConfigCache, it has to go here since we have a sort
 // of circular dependency. Note that we always parse / get the configuration
@@ -1269,11 +1303,7 @@ TSRemapNewInstance(int argc, char *argv[], void **ih, 
char * /* errbuf ATS_UNUSE
 void
 TSRemapDeleteInstance(void *ih)
 {
-  S3Config *s3           = static_cast<S3Config *>(ih);
-  TSMutex   s3_mutex_ptr = s3->config_reloader_mutex();
-  TSMutexLock(s3_mutex_ptr);
-  delete s3;
-  TSMutexUnlock(s3_mutex_ptr);
+  S3Config::deletePluginInstance(static_cast<S3Config *>(ih));
 }
 
 ///////////////////////////////////////////////////////////////////////////////