trafficserver-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sc...@apache.org
Subject [trafficserver] branch master updated: Adjust the refcounts to avoid Mutex leak
Date Wed, 04 Dec 2019 02:38:32 GMT
This is an automated email from the ASF dual-hosted git repository.

scw00 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 4a503c8  Adjust the refcounts to avoid Mutex leak
4a503c8 is described below

commit 4a503c817766fcad3b9eab3ff3a8e9e73d483ae7
Author: Susan Hinrichs <shinrich@oath.com>
AuthorDate: Tue Nov 26 17:06:30 2019 +0000

    Adjust the refcounts to avoid Mutex leak
---
 iocore/eventsystem/I_Lock.h        | 40 +++++++++++++++++++++++++++++++++++---
 src/traffic_server/InkIOCoreAPI.cc | 19 ++++++++++--------
 2 files changed, 48 insertions(+), 11 deletions(-)

diff --git a/iocore/eventsystem/I_Lock.h b/iocore/eventsystem/I_Lock.h
index 4850199..c5b8373 100644
--- a/iocore/eventsystem/I_Lock.h
+++ b/iocore/eventsystem/I_Lock.h
@@ -256,7 +256,7 @@ Mutex_trylock(
 #ifdef DEBUG
   const SourceLocation &location, const char *ahandler,
 #endif
-  Ptr<ProxyMutex> &m, EThread *t)
+  ProxyMutex *m, EThread *t)
 {
   ink_assert(t != nullptr);
   ink_assert(t == reinterpret_cast<EThread *>(this_thread()));
@@ -295,12 +295,26 @@ Mutex_trylock(
   return true;
 }
 
+inline bool
+Mutex_trylock(
+#ifdef DEBUG
+  const SourceLocation &location, const char *ahandler,
+#endif
+  Ptr<ProxyMutex> &m, EThread *t)
+{
+  return Mutex_trylock(
+#ifdef DEBUG
+    location, ahandler,
+#endif
+    m.get(), t);
+}
+
 inline int
 Mutex_lock(
 #ifdef DEBUG
   const SourceLocation &location, const char *ahandler,
 #endif
-  Ptr<ProxyMutex> &m, EThread *t)
+  ProxyMutex *m, EThread *t)
 {
   ink_assert(t != nullptr);
   if (m->thread_holding != t) {
@@ -327,8 +341,22 @@ Mutex_lock(
   return true;
 }
 
+inline int
+Mutex_lock(
+#ifdef DEBUG
+  const SourceLocation &location, const char *ahandler,
+#endif
+  Ptr<ProxyMutex> &m, EThread *t)
+{
+  return Mutex_lock(
+#ifdef DEBUG
+    location, ahandler,
+#endif
+    m.get(), t);
+}
+
 inline void
-Mutex_unlock(Ptr<ProxyMutex> &m, EThread *t)
+Mutex_unlock(ProxyMutex *m, EThread *t)
 {
   if (m->nthread_holding) {
     ink_assert(t == m->thread_holding);
@@ -351,6 +379,12 @@ Mutex_unlock(Ptr<ProxyMutex> &m, EThread *t)
   }
 }
 
+inline void
+Mutex_unlock(Ptr<ProxyMutex> &m, EThread *t)
+{
+  Mutex_unlock(m.get(), t);
+}
+
 class WeakMutexLock
 {
 private:
diff --git a/src/traffic_server/InkIOCoreAPI.cc b/src/traffic_server/InkIOCoreAPI.cc
index 25618ef..1740ee1 100644
--- a/src/traffic_server/InkIOCoreAPI.cc
+++ b/src/traffic_server/InkIOCoreAPI.cc
@@ -235,14 +235,17 @@ TSEventThreadSelf(void)
 
 ////////////////////////////////////////////////////////////////////
 //
-// Mutexes
+// Mutexes:  For TSMutexCreate and TSMutexDestroy, the refcount of the
+// ProxyMutex object is not incremented or decremented.  If the resulting
+// ProxyMutex is passed to a INKContInternal, it's mutex smart pointer
+// will take ownership of the ProxyMutex and delete it when the last
+// reference is removed.  TSMutexDestroy should not be called in that case.
 //
 ////////////////////////////////////////////////////////////////////
 TSMutex
 TSMutexCreate()
 {
   ProxyMutex *mutexp = new_ProxyMutex();
-  mutexp->refcount_inc();
 
   // TODO: Remove this when allocations can never fail.
   sdk_assert(sdk_sanity_check_mutex((TSMutex)mutexp) == TS_SUCCESS);
@@ -255,9 +258,9 @@ TSMutexDestroy(TSMutex m)
 {
   sdk_assert(sdk_sanity_check_mutex(m) == TS_SUCCESS);
   ProxyMutex *mutexp = reinterpret_cast<ProxyMutex *>(m);
-  // Decrement the refcount added in TSMutexCreate.  Delete if this
-  // was the last ref count
-  if (mutexp && mutexp->refcount_dec() == 0) {
+
+  if (mutexp) {
+    ink_release_assert(mutexp->refcount() == 0);
     mutexp->free();
   }
 }
@@ -296,7 +299,7 @@ void
 TSMutexLock(TSMutex mutexp)
 {
   sdk_assert(sdk_sanity_check_mutex(mutexp) == TS_SUCCESS);
-  Ptr<ProxyMutex> proxy_mutex(reinterpret_cast<ProxyMutex *>(mutexp));
+  ProxyMutex *proxy_mutex = reinterpret_cast<ProxyMutex *>(mutexp);
   MUTEX_TAKE_LOCK(proxy_mutex, this_ethread());
 }
 
@@ -304,7 +307,7 @@ TSReturnCode
 TSMutexLockTry(TSMutex mutexp)
 {
   sdk_assert(sdk_sanity_check_mutex(mutexp) == TS_SUCCESS);
-  Ptr<ProxyMutex> proxy_mutex(reinterpret_cast<ProxyMutex *>(mutexp));
+  ProxyMutex *proxy_mutex = reinterpret_cast<ProxyMutex *>(mutexp);
   return (MUTEX_TAKE_TRY_LOCK(proxy_mutex, this_ethread()) ? TS_SUCCESS : TS_ERROR);
 }
 
@@ -312,7 +315,7 @@ void
 TSMutexUnlock(TSMutex mutexp)
 {
   sdk_assert(sdk_sanity_check_mutex(mutexp) == TS_SUCCESS);
-  Ptr<ProxyMutex> proxy_mutex(reinterpret_cast<ProxyMutex *>(mutexp));
+  ProxyMutex *proxy_mutex(reinterpret_cast<ProxyMutex *>(mutexp));
   MUTEX_UNTAKE_LOCK(proxy_mutex, this_ethread());
 }
 


Mime
View raw message