Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions include/tscore/DbgCtl.h
Original file line number Diff line number Diff line change
Expand Up @@ -59,4 +59,27 @@ class DbgCtl
friend TSDbgCtl const *TSDbgCtlCreate(char const *tag);

friend void TSDbgCtlDestroy(TSDbgCtl const *dbg_ctl);

public:
// When loading an ATS plugin with dlopen(), an instance of this class should exist in the stack. This will prevent
// https://github.com/apache/trafficserver/issues/10129 . It will prevent DbgCtl member functions from indiredtly
// calling Regex::compile(), which calls a function that defines a thread_local variable. Such functions try to lock
// the same global mutex (in the C/C++ runtime) that is locked when dlopen() is in progress. This prevents a deadlock
// where shared library static initialization is holding the global mutex and waiting on the Registry mutex, and
// a DbgCtl intanstantiantion has called Regex::compile() in a different thread, is holding the Registry mutex, and
// waiting on the global mutex.
//
class Guard_dlopen
{
public:
Guard_dlopen();
~Guard_dlopen();

// No copying.
Guard_dlopen(Guard_dlopen const &) = delete;
Guard_dlopen &operator=(Guard_dlopen const &) = delete;

private:
_RegistryAccessor *rap;
};
};
8 changes: 6 additions & 2 deletions proxy/Plugin.cc
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
#include "Plugin.h"
#include "tscore/ink_cap.h"
#include "tscore/Filenames.h"
#include "tscore/DbgCtl.h"

#define MAX_PLUGIN_ARGS 64

Expand Down Expand Up @@ -102,8 +103,11 @@ PluginRegInfo::~PluginRegInfo()
bool
plugin_dso_load(const char *path, void *&handle, void *&init, std::string &error)
{
handle = dlopen(path, RTLD_NOW);
init = nullptr;
{
DbgCtl::Guard_dlopen g;
handle = dlopen(path, RTLD_NOW);
}
init = nullptr;
if (!handle) {
error.assign("unable to load '").append(path).append("': ").append(dlerror());
Error("%s", error.c_str());
Expand Down
12 changes: 9 additions & 3 deletions proxy/http/remap/PluginDso.cc
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
#include "unit-tests/plugin_testing_common.h"
#else
#include "tscore/Diags.h"
#include "tscore/DbgCtl.h"
#define PluginDebug Debug
#define PluginError Error
#endif
Expand Down Expand Up @@ -105,10 +106,15 @@ PluginDso::load(std::string &error)

/* Now attempt to load the plugin DSO */
#if defined(darwin)
if (!dlopen_preflight(_runtimePath.c_str()) || (_dlh = dlopen(_runtimePath.c_str(), RTLD_NOW | RTLD_LOCAL)) == nullptr) {
#else
if ((_dlh = dlopen(_runtimePath.c_str(), RTLD_NOW | RTLD_LOCAL)) == nullptr) {
if (!dlopen_preflight(_runtimePath.c_str())) {
_dlh = nullptr;
} else
#endif
{
DbgCtl::Guard_dlopen g;
_dlh = dlopen(_runtimePath.c_str(), RTLD_NOW | RTLD_LOCAL);
}
if (_dlh == nullptr) {
const char *err = dlerror();
concat_error(error, err ? err : "unknown dlopen() error");
_dlh = nullptr; /* mark that the constructor failed. */
Expand Down
9 changes: 8 additions & 1 deletion src/tscore/DbgCtl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ class DbgCtl::_RegistryAccessor
_mtx.unlock();
}

std::mutex _mtx;
std::recursive_mutex _mtx;

friend class DbgCtl::_RegistryAccessor;
};
Expand Down Expand Up @@ -195,3 +195,10 @@ DbgCtl::update()
const_cast<char volatile &>(i.on) = diags()->tag_activated(i.tag, DiagsTagType_Debug);
}
}

DbgCtl::Guard_dlopen::Guard_dlopen() : rap{new _RegistryAccessor} {};

DbgCtl::Guard_dlopen::~Guard_dlopen()
{
delete rap;
}