Skip to content

Commit 6e812e3

Browse files
authored
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
1 parent 624462f commit 6e812e3

1 file changed

Lines changed: 35 additions & 5 deletions

File tree

plugins/origin_server_auth/origin_server_auth.cc

Lines changed: 35 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@
4848
#include <ts/remap.h>
4949
#include <ts/remap_version.h>
5050
#include <tsutil/TsSharedMutex.h>
51+
#include <utility>
5152
#include "ts/apidefs.h"
5253
#include "tscore/ink_config.h"
5354
#include "swoc/TextView.h"
@@ -224,6 +225,16 @@ class S3Config
224225
}
225226
}
226227

228+
// This handles the hoops needed to safely delete the special
229+
// plugin level instance of this class.
230+
static void deletePluginInstance(S3Config *p);
231+
232+
TSCont
233+
releaseReloadCont()
234+
{
235+
return std::exchange(_conf_rld, nullptr);
236+
}
237+
227238
// Is this configuration usable?
228239
bool
229240
valid() const
@@ -647,6 +658,29 @@ S3Config::parse_config(const std::string &config_fname)
647658
return true;
648659
}
649660

661+
void
662+
S3Config::deletePluginInstance(S3Config *s3)
663+
{
664+
// Due to a race conditions with the background continuation
665+
// the plugin shutdown has to take care not to run
666+
// when the reload continuation does.
667+
//
668+
// The order of oprations here is important, be careful making changes
669+
if (s3) {
670+
TSMutex s3_mutex_ptr = s3->config_reloader_mutex();
671+
672+
TSMutexLock(s3_mutex_ptr);
673+
auto cont = s3->releaseReloadCont();
674+
delete s3;
675+
TSMutexUnlock(s3_mutex_ptr);
676+
// Its important to only destory this continuation outside of holding
677+
// its lock, but after it has been cancelled (in the S3Config destructor)
678+
if (cont != nullptr) {
679+
TSContDestroy(cont);
680+
}
681+
}
682+
}
683+
650684
///////////////////////////////////////////////////////////////////////////////
651685
// Implementation for the ConfigCache, it has to go here since we have a sort
652686
// 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
12691303
void
12701304
TSRemapDeleteInstance(void *ih)
12711305
{
1272-
S3Config *s3 = static_cast<S3Config *>(ih);
1273-
TSMutex s3_mutex_ptr = s3->config_reloader_mutex();
1274-
TSMutexLock(s3_mutex_ptr);
1275-
delete s3;
1276-
TSMutexUnlock(s3_mutex_ptr);
1306+
S3Config::deletePluginInstance(static_cast<S3Config *>(ih));
12771307
}
12781308

12791309
///////////////////////////////////////////////////////////////////////////////

0 commit comments

Comments
 (0)