From fb569224483d90b65a519a5470e5276ae0be2730 Mon Sep 17 00:00:00 2001 From: Walt Karas Date: Mon, 13 Jun 2022 18:45:36 -0500 Subject: [PATCH] Add an ink_mutex destroy function that tolerates the mutex being locked. Also converts more calls from strerror() to thread-safe ts::Strerror class. --- include/tscore/ink_mutex.h | 17 ++++++-- include/tscpp/util/TsSharedMutex.h | 1 + iocore/eventsystem/I_Lock.h | 2 +- src/tscore/Makefile.am | 1 + src/tscore/ink_mutex.cc | 12 +++--- src/tscore/ink_mutex_sd.cc | 63 ++++++++++++++++++++++++++++++ 6 files changed, 84 insertions(+), 12 deletions(-) create mode 100644 src/tscore/ink_mutex_sd.cc diff --git a/include/tscore/ink_mutex.h b/include/tscore/ink_mutex.h index 025e69fab9d..e2eb5bbc500 100644 --- a/include/tscore/ink_mutex.h +++ b/include/tscore/ink_mutex.h @@ -32,22 +32,31 @@ ***********************************************************************/ -#include "tscore/ink_defs.h" -#include "tscore/ink_error.h" +#include +#include +#include #include #include typedef pthread_mutex_t ink_mutex; void ink_mutex_init(ink_mutex *m); + +// This will abort if the mutex is locked. Has better performance than ink_mutex_safer_destroy(). +// void ink_mutex_destroy(ink_mutex *m); +// In a relesae build, if the mutex is locked, this will log a warning, then wait up to 10 seconds for the mutex to be +// unlocked by another thread. In a debug build, the behavior is essentially the same as ink_mutex_destroy(). +// +void ink_mutex_safer_destroy(ink_mutex *m); + static inline void ink_mutex_acquire(ink_mutex *m) { int error = pthread_mutex_lock(m); if (unlikely(error != 0)) { - ink_abort("pthread_mutex_lock(%p) failed: %s (%d)", m, strerror(error), error); + ink_abort("pthread_mutex_lock(%p) failed: %s (%d)", m, ts::Strerror(error).c_str(), error); } } @@ -56,7 +65,7 @@ ink_mutex_release(ink_mutex *m) { int error = pthread_mutex_unlock(m); if (unlikely(error != 0)) { - ink_abort("pthread_mutex_unlock(%p) failed: %s (%d)", m, strerror(error), error); + ink_abort("pthread_mutex_unlock(%p) failed: %s (%d)", m, ts::Strerror(error).c_str(), error); } } diff --git a/include/tscpp/util/TsSharedMutex.h b/include/tscpp/util/TsSharedMutex.h index df0fd2c23ee..fedf7dcbfe8 100644 --- a/include/tscpp/util/TsSharedMutex.h +++ b/include/tscpp/util/TsSharedMutex.h @@ -25,6 +25,7 @@ #pragma once #include + #include #if __has_include() diff --git a/iocore/eventsystem/I_Lock.h b/iocore/eventsystem/I_Lock.h index 7efa45f462c..94d99f174d9 100644 --- a/iocore/eventsystem/I_Lock.h +++ b/iocore/eventsystem/I_Lock.h @@ -592,7 +592,7 @@ ProxyMutex::free() print_lock_stats(1); #endif #endif - ink_mutex_destroy(&the_mutex); + ink_mutex_safer_destroy(&the_mutex); mutexAllocator.free(this); } diff --git a/src/tscore/Makefile.am b/src/tscore/Makefile.am index 80fbb8f7785..fbaad6b2382 100644 --- a/src/tscore/Makefile.am +++ b/src/tscore/Makefile.am @@ -84,6 +84,7 @@ libtscore_la_SOURCES = \ ink_inet.cc \ ink_memory.cc \ ink_mutex.cc \ + ink_mutex_sd.cc \ ink_queue.cc \ ink_queue_utils.cc \ ink_rand.cc \ diff --git a/src/tscore/ink_mutex.cc b/src/tscore/ink_mutex.cc index 606359b3b9d..37d10119eb3 100644 --- a/src/tscore/ink_mutex.cc +++ b/src/tscore/ink_mutex.cc @@ -21,11 +21,9 @@ limitations under the License. */ -#include "tscore/ink_error.h" -#include "tscore/ink_defs.h" -#include -#include -#include "tscore/ink_mutex.h" +#include +#include +#include ink_mutex __global_death = PTHREAD_MUTEX_INITIALIZER; @@ -58,7 +56,7 @@ ink_mutex_init(ink_mutex *m) error = pthread_mutex_init(m, &attr.attr); if (unlikely(error != 0)) { - ink_abort("pthread_mutex_init(%p) failed: %s (%d)", m, strerror(error), error); + ink_abort("pthread_mutex_init(%p) failed: %s (%d)", m, ts::Strerror(error).c_str(), error); } } @@ -69,6 +67,6 @@ ink_mutex_destroy(ink_mutex *m) error = pthread_mutex_destroy(m); if (unlikely(error != 0)) { - ink_abort("pthread_mutex_destroy(%p) failed: %s (%d)", m, strerror(error), error); + ink_abort("pthread_mutex_destroy(%p) failed: %s (%d)", m, ts::Strerror(error).c_str(), error); } } diff --git a/src/tscore/ink_mutex_sd.cc b/src/tscore/ink_mutex_sd.cc new file mode 100644 index 00000000000..dbfd535c536 --- /dev/null +++ b/src/tscore/ink_mutex_sd.cc @@ -0,0 +1,63 @@ +/** @file + + A brief file description + + @section license License + + Licensed to the Apache Software Foundation (ASF) under one + or more contributor license agreements. See the NOTICE file + distributed with this work for additional information + regarding copyright ownership. The ASF licenses this file + to you under the Apache License, Version 2.0 (the + "License"); you may not use this file except in compliance + with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + */ + +#include +#include +#include +#include + +// This is in a separate module from ink_mutex because some TS tools don't need this function, and also do not link in +// the Diags objects. So putting this in ink_mutex causes a link error for these tools for the Warning() call. + +void +ink_mutex_safer_destroy(ink_mutex *m) +{ + int error; + + error = pthread_mutex_trylock(m); + if (unlikely(error != 0)) { + if (error != EBUSY) { + ink_abort("pthread_mutex_trylock(%p) failed: %s (%d)", m, ts::Strerror(error).c_str(), error); + } else { +#if DEBUG + ink_abort("destroying mutex (%p) that is still locked", m); +#else + Warning("ink_mutex_safer_destroy: destroying mutex (%p) that is still locked", m); +#endif + + timespec timeout; + timeout.tv_sec = 10; + timeout.tv_nsec = 0; + error = pthread_mutex_timedlock(m, &timeout); + if (error != 0) { + ink_abort("pthread_mutex_trylock(%p) failed: %s (%d)", m, ts::Strerror(error).c_str(), error); + } + } + } + ink_mutex_release(m); + + error = pthread_mutex_destroy(m); + if (unlikely(error != 0)) { + ink_abort("pthread_mutex_destroy(%p) failed: %s (%d)", m, ts::Strerror(error).c_str(), error); + } +}