Mercurial > hg > heapstats

changeset 277:ebbca28d3efd

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
Bug 3765: pthread mutex lock/unlock enhancement Reviewed-by: ykubota https://github.com/HeapStats/heapstats/pull/150
author Yasumasa Suenaga <yasuenag@gmail.com>
date Sun, 10 Nov 2019 00:25:20 +0900
parents 5d5beb9883df
children 59c5f8aba0b5
files ChangeLog agent/src/heapstats-engines/agentThread.cpp agent/src/heapstats-engines/deadlockDetector.cpp agent/src/heapstats-engines/fsUtil.cpp agent/src/heapstats-engines/gcWatcher.cpp agent/src/heapstats-engines/logMain.cpp agent/src/heapstats-engines/logManager.cpp agent/src/heapstats-engines/snapShotMain.cpp agent/src/heapstats-engines/snapShotProcessor.cpp agent/src/heapstats-engines/timer.cpp agent/src/heapstats-engines/trapSender.cpp agent/src/heapstats-engines/util.hpp
diffstat 12 files changed, 107 insertions(+), 115 deletions(-) [+]
line wrap: on
line diff
--- a/ChangeLog	Tue Oct 08 23:12:56 2019 +0900
+++ b/ChangeLog	Sun Nov 10 00:25:20 2019 +0900
@@ -1,3 +1,7 @@
+2019-11-10 Yasumasa Suenaga <yasuenag@gmail.com>
+
+	* Bug 3765: pthread mutex lock/unlock enhancement
+
 2019-10-08 Yasumasa Suenaga <yasuenag@gmail.com>
 
 	* Bug 3759: HeapStats Agent could not be built on CentOS 6
--- a/agent/src/heapstats-engines/agentThread.cpp	Tue Oct 08 23:12:56 2019 +0900
+++ b/agent/src/heapstats-engines/agentThread.cpp	Sun Nov 10 00:25:20 2019 +0900
@@ -1,7 +1,7 @@
 /*!
  * \file agentThread.cpp
  * \brief This file is used to work on Jthread.
- * Copyright (C) 2011-2015 Nippon Telegraph and Telephone Corporation
+ * Copyright (C) 2011-2019 Nippon Telegraph and Telephone Corporation
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License
@@ -108,11 +108,10 @@
  */
 void TAgentThread::notify(void) {
   /* Send notification and count notify. */
-  ENTER_PTHREAD_SECTION(&this->mutex) {
-    this->_numRequests++;
-    pthread_cond_signal(&this->mutexCond);
-  }
-  EXIT_PTHREAD_SECTION(&this->mutex)
+  TMutexLocker locker(&this->mutex);
+
+  this->_numRequests++;
+  pthread_cond_signal(&this->mutexCond);
 }
 
 /*!
@@ -126,11 +125,12 @@
   }
 
   /* Send notification and count notify. */
-  ENTER_PTHREAD_SECTION(&this->mutex) {
+  {
+    TMutexLocker locker(&this->mutex);
+
     this->_terminateRequest = true;
     pthread_cond_signal(&this->mutexCond);
   }
-  EXIT_PTHREAD_SECTION(&this->mutex)
 
   /* SpinLock for AgentThread termination. */
   while (this->_isRunning) {
--- a/agent/src/heapstats-engines/deadlockDetector.cpp	Tue Oct 08 23:12:56 2019 +0900
+++ b/agent/src/heapstats-engines/deadlockDetector.cpp	Sun Nov 10 00:25:20 2019 +0900
@@ -1,7 +1,7 @@
 /*!
  * \file deadlockDetector.cpp
  * \brief This file is used by find deadlock.
- * Copyright (C) 2017 Yasumasa Suenaga
+ * Copyright (C) 2017-2019 Yasumasa Suenaga
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License
@@ -181,8 +181,9 @@
     jvmti->GetObjectHashCode(thread, &thread_hash);
     jvmti->GetObjectHashCode(object, &monitor_hash);
 
-    ENTER_PTHREAD_SECTION(&mutex);
     {
+      TMutexLocker locker(&mutex);
+
       /* Avoid JDK-8185164 */
       bool canSkip = true;
 
@@ -233,7 +234,6 @@
         }
       }
     }
-    EXIT_PTHREAD_SECTION(&mutex);
   }
 
 
@@ -262,8 +262,9 @@
       return;
     }
 
-    ENTER_PTHREAD_SECTION(&mutex);
     {
+      TMutexLocker locker(&mutex);
+
       /* Remove all owned monitors from owner list */
       for (int idx = 0; idx < monitor_cnt; idx++) {
         jint monitor_hash;
@@ -277,7 +278,6 @@
       jvmti->GetObjectHashCode(thread, &thread_hash);
       waiter_list.erase(thread_hash);
     }
-    EXIT_PTHREAD_SECTION(&mutex);
   }
 
 
@@ -343,12 +343,11 @@
       sched_yield();
     }
 
-    ENTER_PTHREAD_SECTION(&mutex);
     {
+      TMutexLocker locker(&mutex);
       monitor_owners.clear();
       waiter_list.clear();
     }
-    EXIT_PTHREAD_SECTION(&mutex);
   }
 
 }
--- a/agent/src/heapstats-engines/fsUtil.cpp	Tue Oct 08 23:12:56 2019 +0900
+++ b/agent/src/heapstats-engines/fsUtil.cpp	Sun Nov 10 00:25:20 2019 +0900
@@ -1,7 +1,7 @@
 /*!
  * \file fsUtil.cpp
  * \brief This file is utilities to access file system.
- * Copyright (C) 2011-2018 Nippon Telegraph and Telephone Corporation
+ * Copyright (C) 2011-2019 Nippon Telegraph and Telephone Corporation
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License
@@ -242,8 +242,8 @@
   }
 
   int raisedErrNum = -1;
-  /* Get mutex. */
-  ENTER_PTHREAD_SECTION(&directoryMutex) {
+  {
+    TMutexLocker locker(&directoryMutex);
 
     /* Create unique directory path. */
     uniqName = createUniquePath((char *)wishesName, true);
@@ -258,8 +258,6 @@
       }
     }
   }
-  /* Release mutex. */
-  EXIT_PTHREAD_SECTION(&directoryMutex)
 
   /* If failed to create temporary directory. */
   if (unlikely(raisedErrNum != 0)) {
@@ -330,8 +328,8 @@
   /* Cleanup. */
   closedir(dir);
 
-  /* Get mutex. */
-  ENTER_PTHREAD_SECTION(&directoryMutex) {
+  {
+    TMutexLocker locker(&directoryMutex);
 
     /* Remove directory. */
     if (unlikely(rmdir(basePath) != 0)) {
@@ -340,8 +338,6 @@
     }
 
   }
-  /* Release mutex. */
-  EXIT_PTHREAD_SECTION(&directoryMutex)
 }
 
 /*!
--- a/agent/src/heapstats-engines/gcWatcher.cpp	Tue Oct 08 23:12:56 2019 +0900
+++ b/agent/src/heapstats-engines/gcWatcher.cpp	Sun Nov 10 00:25:20 2019 +0900
@@ -1,7 +1,7 @@
 /*!
  * \file gcWatcher.cpp
  * \brief This file is used to take snapshot when finish garbage collection.
- * Copyright (C) 2011-2017 Nippon Telegraph and Telephone Corporation
+ * Copyright (C) 2011-2019 Nippon Telegraph and Telephone Corporation
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License
@@ -68,7 +68,9 @@
     /* Variable for notification flag. */
     bool needProcess = false;
 
-    ENTER_PTHREAD_SECTION(&controller->mutex) {
+    {
+      TMutexLocker locker(&controller->mutex);
+
       /* If no exists request. */
       if (likely(controller->_numRequests == 0)) {
         /* Wait for notification or termination. */
@@ -82,7 +84,6 @@
         needProcess = true;
       }
     }
-    EXIT_PTHREAD_SECTION(&controller->mutex)
 
     /* If waiting finished by notification. */
     if (needProcess) {
--- a/agent/src/heapstats-engines/logMain.cpp	Tue Oct 08 23:12:56 2019 +0900
+++ b/agent/src/heapstats-engines/logMain.cpp	Sun Nov 10 00:25:20 2019 +0900
@@ -1,7 +1,7 @@
 /*!
  * \file logmain.cpp
  * \brief This file is used common logging process.
- * Copyright (C) 2011-2017 Nippon Telegraph and Telephone Corporation
+ * Copyright (C) 2011-2019 Nippon Telegraph and Telephone Corporation
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License
@@ -226,8 +226,8 @@
   }
 
   bool isCollectLog = true;
-  /* Lock to use in multi-thread. */
-  ENTER_PTHREAD_SECTION(&errMutex) {
+  {
+    TMutexLocker locker(&errMutex);
 
     /* If collected already and collect first only. */
     if (conf->FirstCollect()->get() && conf->isFirstCollected()) {
@@ -238,8 +238,6 @@
     conf->setFirstCollected(true);
 
   }
-  /* Unlock to use in multi-thread. */
-  EXIT_PTHREAD_SECTION(&errMutex)
 
   if (isCollectLog) {
     /* Setting collect log cause. */
--- a/agent/src/heapstats-engines/logManager.cpp	Tue Oct 08 23:12:56 2019 +0900
+++ b/agent/src/heapstats-engines/logManager.cpp	Sun Nov 10 00:25:20 2019 +0900
@@ -1,7 +1,7 @@
 /*!
  * \file logManager.cpp
  * \brief This file is used collect log information.
- * Copyright (C) 2011-2018 Nippon Telegraph and Telephone Corporation
+ * Copyright (C) 2011-2019 Nippon Telegraph and Telephone Corporation
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License
@@ -255,8 +255,8 @@
            /* Params : Archive file name. */
            archivePath);
 
-  /* Get mutex. */
-  ENTER_PTHREAD_SECTION(&logMutex) {
+  {
+    TMutexLocker locker(&logMutex);
 
     /* Open log file. */
     int fd = open(conf->HeapLogFile()->get(), O_CREAT | O_WRONLY | O_APPEND,
@@ -282,8 +282,7 @@
       }
     }
   }
-  /* Release mutex. */
-  EXIT_PTHREAD_SECTION(&logMutex)
+
   return result;
 }
 
@@ -379,8 +378,8 @@
      */
     result = -1;
 
-    /* Get mutex. */
-    ENTER_PTHREAD_SECTION(&archiveMutex) {
+    {
+      TMutexLocker locker(&archiveMutex);
 
       /* Create archive file name. */
       uniqArcName = createArchiveName(nowTime);
@@ -400,8 +399,6 @@
         }
       }
     }
-    /* Release mutex. */
-    EXIT_PTHREAD_SECTION(&archiveMutex)
 
     /* If failure create archive file yet. */
     if (unlikely(result != 0)) {
--- a/agent/src/heapstats-engines/snapShotMain.cpp	Tue Oct 08 23:12:56 2019 +0900
+++ b/agent/src/heapstats-engines/snapShotMain.cpp	Sun Nov 10 00:25:20 2019 +0900
@@ -1,7 +1,7 @@
 /*!
  * \file snapShotMain.cpp
  * \brief This file is used to take snapshot.
- * Copyright (C) 2011-2017 Nippon Telegraph and Telephone Corporation
+ * Copyright (C) 2011-2019 Nippon Telegraph and Telephone Corporation
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License
@@ -546,15 +546,12 @@
  */
 void JNICALL OnDataDumpRequestForSnapShot(jvmtiEnv *jvmti) {
   TProcessMark mark(processing);
+  TMutexLocker locker(&dumpMutex);
 
   /* Avoid the plural simultaneous take snapshot by dump-request.        */
   /* E.g. keeping pushed dump key.                                       */
   /* Because classContainer register a redundancy class in TakeSnapShot. */
-  ENTER_PTHREAD_SECTION(&dumpMutex) {
-    /* Make snapshot. */
-    TakeSnapShot(jvmti, NULL, DataDumpRequest);
-  }
-  EXIT_PTHREAD_SECTION(&dumpMutex)
+  TakeSnapShot(jvmti, NULL, DataDumpRequest);
 }
 
 /*!
@@ -595,22 +592,21 @@
 
     if (likely(snapshot != NULL)) {
       /* Lock to avoid doubling call JVMTI. */
-      ENTER_PTHREAD_SECTION(&jvmtiMutex) {
-        snapshotByJvmti = snapshot;
+      TMutexLocker locker(&jvmtiMutex);
+
+      snapshotByJvmti = snapshot;
 
-        /* Enable JVMTI hooking. */
-        if (likely(setJvmtiHookState(true))) {
-          /* Count object size on heap. */
-          error = jvmti->IterateOverHeap(JVMTI_HEAP_OBJECT_EITHER,
-                                         &HeapObjectCallBack, NULL);
+      /* Enable JVMTI hooking. */
+      if (likely(setJvmtiHookState(true))) {
+        /* Count object size on heap. */
+        error = jvmti->IterateOverHeap(JVMTI_HEAP_OBJECT_EITHER,
+                                       &HeapObjectCallBack, NULL);
 
-          /* Disable JVMTI hooking. */
-          setJvmtiHookState(false);
-        }
+        /* Disable JVMTI hooking. */
+        setJvmtiHookState(false);
+      }
 
-        snapshotByJvmti = NULL;
-      }
-      EXIT_PTHREAD_SECTION(&jvmtiMutex)
+      snapshotByJvmti = NULL;
     }
 
     if (likely(error == JVMTI_ERROR_NONE)) {
--- a/agent/src/heapstats-engines/snapShotProcessor.cpp	Tue Oct 08 23:12:56 2019 +0900
+++ b/agent/src/heapstats-engines/snapShotProcessor.cpp	Sun Nov 10 00:25:20 2019 +0900
@@ -1,7 +1,7 @@
 /*!
  * \file snapShotProcessor.cpp
  * \brief This file is used to output ranking and call snapshot function.
- * Copyright (C) 2011-2017 Nippon Telegraph and Telephone Corporation
+ * Copyright (C) 2011-2019 Nippon Telegraph and Telephone Corporation
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License
@@ -76,7 +76,9 @@
     /* Is notify flag. */
     bool needProcess = false;
 
-    ENTER_PTHREAD_SECTION(&controller->mutex) {
+    {
+      TMutexLocker locker(&controller->mutex);
+
       if (likely(controller->_numRequests == 0)) {
         /* Wait for notification or termination. */
         pthread_cond_wait(&controller->mutexCond, &controller->mutex);
@@ -93,7 +95,6 @@
       /* Check remaining work. */
       existRemainder = (controller->_numRequests > 0);
     }
-    EXIT_PTHREAD_SECTION(&controller->mutex)
 
     /* If waiting is finished by notification. */
     if (needProcess && (snapshot != NULL)) {
@@ -158,7 +159,9 @@
 
   bool raiseException = true;
   /* Send notification and count notify. */
-  ENTER_PTHREAD_SECTION(&this->mutex) {
+  {
+    TMutexLocker locker(&this->mutex);
+
     try {
       /* Store and count data. */
       snapQueue.push(snapshot);
@@ -176,7 +179,6 @@
        */
     }
   }
-  EXIT_PTHREAD_SECTION(&this->mutex)
 
   if (unlikely(raiseException)) {
     throw "Failed to TSnapShotProcessor notify";
--- a/agent/src/heapstats-engines/timer.cpp	Tue Oct 08 23:12:56 2019 +0900
+++ b/agent/src/heapstats-engines/timer.cpp	Sun Nov 10 00:25:20 2019 +0900
@@ -1,7 +1,7 @@
 /*!
  * \file timer.cpp
  * \brief This file is used to take interval snapshot.
- * Copyright (C) 2011-2015 Nippon Telegraph and Telephone Corporation
+ * Copyright (C) 2011-2019 Nippon Telegraph and Telephone Corporation
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License
@@ -76,7 +76,9 @@
     /* Reset timer interrupt flag. */
     controller->_isInterrupted = false;
 
-    ENTER_PTHREAD_SECTION(&controller->mutex) {
+    {
+      TMutexLocker locker(&controller->mutex);
+
       /* Create limit datetime. */
       struct timespec limitTs = {0};
       struct timeval nowTv = {0};
@@ -89,7 +91,6 @@
       pthread_cond_timedwait(&controller->mutexCond, &controller->mutex,
                              &limitTs);
     }
-    EXIT_PTHREAD_SECTION(&controller->mutex)
 
     /* If waiting finished by timeout. */
     if (!controller->_isInterrupted) {
@@ -166,11 +167,10 @@
 
   } else {
     /* Set interrupt flag and notify. */
-    ENTER_PTHREAD_SECTION(&this->mutex) {
-      this->_isInterrupted = true;
-      pthread_cond_signal(&this->mutexCond);
-    }
-    EXIT_PTHREAD_SECTION(&this->mutex)
+    TMutexLocker locker(&this->mutex);
+
+    this->_isInterrupted = true;
+    pthread_cond_signal(&this->mutexCond);
   }
 }
 
--- a/agent/src/heapstats-engines/trapSender.cpp	Tue Oct 08 23:12:56 2019 +0900
+++ b/agent/src/heapstats-engines/trapSender.cpp	Sun Nov 10 00:25:20 2019 +0900
@@ -1,7 +1,7 @@
 /*!
  * \file trapSender.cpp
  * \brief This file is used to send SNMP trap.
- * Copyright (C) 2016-2017 Yasumasa Suenaga
+ * Copyright (C) 2016-2019 Yasumasa Suenaga
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License
@@ -96,7 +96,9 @@
   }
 
   /* Initialize session. */
-  ENTER_PTHREAD_SECTION(&senderMutex) {
+  {
+    TMutexLocker locker(&senderMutex);
+
     memset(&session, 0, sizeof(netsnmp_session));
     netSnmpFuncs.snmp_sess_init(&session);
     session.version = snmp;
@@ -104,7 +106,7 @@
     session.remote_port = port;
     session.community = (u_char *)strdup(pCommName);
     session.community_len = (pCommName != NULL) ? strlen(pCommName) : 0;
-  } EXIT_PTHREAD_SECTION(&senderMutex)
+  }
 
   return true;
 }
@@ -114,11 +116,13 @@
  */
 void TTrapSender::finalize(void) {
   /* Close and free SNMP session. */
-  ENTER_PTHREAD_SECTION(&senderMutex) {
+  {
+    TMutexLocker locker(&senderMutex);
+
     netSnmpFuncs.snmp_close(&session);
     free(session.peername);
     free(session.community);
-  } EXIT_PTHREAD_SECTION(&senderMutex)
+  }
 
   /* Unload library */
   if (libnetsnmp_handle != NULL) {
@@ -130,35 +134,28 @@
  * \brief TrapSender constructor.
  */
 TTrapSender::TTrapSender() {
-  /* Lock to use in multi-thread. */
-  ENTER_PTHREAD_SECTION(&senderMutex) {
-    /* Disable NETSNMP logging. */
-    netSnmpFuncs.netsnmp_register_loghandler(
-                               NETSNMP_LOGHANDLER_NONE, LOG_EMERG);
-    /* Make a PDU */
-    pPdu = netSnmpFuncs.snmp_pdu_create(SNMP_MSG_TRAP2);
-  }
-  /* Unlock to use in multi-thread. */
-  EXIT_PTHREAD_SECTION(&senderMutex)
+  TMutexLocker locker(&senderMutex);
+
+  /* Disable NETSNMP logging. */
+  netSnmpFuncs.netsnmp_register_loghandler(
+                             NETSNMP_LOGHANDLER_NONE, LOG_EMERG);
+  /* Make a PDU */
+  pPdu = netSnmpFuncs.snmp_pdu_create(SNMP_MSG_TRAP2);
 }
 
 /*!
  * \brief TrapSender destructor.
  */
 TTrapSender::~TTrapSender(void) {
-  /* Lock to use in multi-thread. */
-  ENTER_PTHREAD_SECTION(&senderMutex) {
-    /* Clear Allocated str. */
-    clearValues();
+  TMutexLocker locker(&senderMutex);
+
+  /* Clear Allocated str. */
+  clearValues();
 
-    /* Free SNMP pdu. */
-    if (pPdu != NULL) {
-      netSnmpFuncs.snmp_free_pdu(pPdu);
-    }
-
+  /* Free SNMP pdu. */
+  if (pPdu != NULL) {
+    netSnmpFuncs.snmp_free_pdu(pPdu);
   }
-  /* Unlock to use in multi-thread. */
-  EXIT_PTHREAD_SECTION(&senderMutex)
 }
 
 /*!
--- a/agent/src/heapstats-engines/util.hpp	Tue Oct 08 23:12:56 2019 +0900
+++ b/agent/src/heapstats-engines/util.hpp	Sun Nov 10 00:25:20 2019 +0900
@@ -1,7 +1,7 @@
 /*!
  * \file util.hpp
  * \brief This file is utilities.
- * Copyright (C) 2011-2017 Nippon Telegraph and Telephone Corporation
+ * Copyright (C) 2011-2019 Nippon Telegraph and Telephone Corporation
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License
@@ -31,9 +31,11 @@
 #include <cstdatomic>
 #endif
 
+#include <assert.h>
 #include <stddef.h>
 #include <errno.h>
 #include <string.h>
+#include <pthread.h>
 
 /* Branch prediction. */
 
@@ -47,23 +49,23 @@
  */
 #define unlikely(x) __builtin_expect(!!(x), 0)
 
-/* Critical section helper macro for pthread mutex. */
-
 /*!
- * \brief Enter critical pthread section macro.
+ * \brief C++ class for pthread mutex.
+ *        C'tor acquires mutex, and d'tor releases it.
  */
-#define ENTER_PTHREAD_SECTION(monitor)                \
-  if (unlikely(pthread_mutex_lock((monitor)) != 0)) { \
-    logger->printWarnMsg("Entering mutex failed!");   \
-  } else {
-/*!
- * \brief Exit critical pthread section macro.
- */
-#define EXIT_PTHREAD_SECTION(monitor)                   \
-  if (unlikely(pthread_mutex_unlock((monitor)) != 0)) { \
-    logger->printWarnMsg("Exiting mutex failed!");      \
-  }                                                     \
-  }
+class TMutexLocker {
+  private:
+    pthread_mutex_t *_mutex;
+
+  public:
+    TMutexLocker(pthread_mutex_t *mutex) : _mutex(mutex) {
+      assert(pthread_mutex_lock(_mutex) == 0);
+    }
+
+    ~TMutexLocker() {
+      assert(pthread_mutex_unlock(_mutex) == 0);
+    }
+};
 
 /*!
  * \brief Calculate align macro.