# HG changeset patch # User Yasumasa Suenaga # Date 1573313167 -32400 # Node ID b7dba538ad10b2dca65080268f16a26448a70cd1 # Parent b037430367a789d509b57aeeea9d119d494e338d Bug 3765: pthread mutex lock/unlock enhancement Reviewed-by: ykubota https://github.com/HeapStats/heapstats/pull/150 diff -r b037430367a7 -r b7dba538ad10 ChangeLog --- a/ChangeLog Thu Jul 25 00:04:53 2019 +0900 +++ b/ChangeLog Sun Nov 10 00:26:07 2019 +0900 @@ -1,3 +1,7 @@ +2018-11-10 Yasumasa Suenaga + + * Bug 3765: pthread mutex lock/unlock enhancement + 2019-07-24 KUBOTA Yuji * Bug 3747: Add note to avoid misrecognized about ResourceExhausted diff -r b037430367a7 -r b7dba538ad10 agent/src/heapstats-engines/agentThread.cpp --- a/agent/src/heapstats-engines/agentThread.cpp Thu Jul 25 00:04:53 2019 +0900 +++ b/agent/src/heapstats-engines/agentThread.cpp Sun Nov 10 00:26:07 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) { diff -r b037430367a7 -r b7dba538ad10 agent/src/heapstats-engines/deadlockDetector.cpp --- a/agent/src/heapstats-engines/deadlockDetector.cpp Thu Jul 25 00:04:53 2019 +0900 +++ b/agent/src/heapstats-engines/deadlockDetector.cpp Sun Nov 10 00:26:07 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); } } diff -r b037430367a7 -r b7dba538ad10 agent/src/heapstats-engines/fsUtil.cpp --- a/agent/src/heapstats-engines/fsUtil.cpp Thu Jul 25 00:04:53 2019 +0900 +++ b/agent/src/heapstats-engines/fsUtil.cpp Sun Nov 10 00:26:07 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) } /*! diff -r b037430367a7 -r b7dba538ad10 agent/src/heapstats-engines/gcWatcher.cpp --- a/agent/src/heapstats-engines/gcWatcher.cpp Thu Jul 25 00:04:53 2019 +0900 +++ b/agent/src/heapstats-engines/gcWatcher.cpp Sun Nov 10 00:26:07 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) { diff -r b037430367a7 -r b7dba538ad10 agent/src/heapstats-engines/logMain.cpp --- a/agent/src/heapstats-engines/logMain.cpp Thu Jul 25 00:04:53 2019 +0900 +++ b/agent/src/heapstats-engines/logMain.cpp Sun Nov 10 00:26:07 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. */ diff -r b037430367a7 -r b7dba538ad10 agent/src/heapstats-engines/logManager.cpp --- a/agent/src/heapstats-engines/logManager.cpp Thu Jul 25 00:04:53 2019 +0900 +++ b/agent/src/heapstats-engines/logManager.cpp Sun Nov 10 00:26:07 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)) { diff -r b037430367a7 -r b7dba538ad10 agent/src/heapstats-engines/snapShotMain.cpp --- a/agent/src/heapstats-engines/snapShotMain.cpp Thu Jul 25 00:04:53 2019 +0900 +++ b/agent/src/heapstats-engines/snapShotMain.cpp Sun Nov 10 00:26:07 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)) { diff -r b037430367a7 -r b7dba538ad10 agent/src/heapstats-engines/snapShotProcessor.cpp --- a/agent/src/heapstats-engines/snapShotProcessor.cpp Thu Jul 25 00:04:53 2019 +0900 +++ b/agent/src/heapstats-engines/snapShotProcessor.cpp Sun Nov 10 00:26:07 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"; diff -r b037430367a7 -r b7dba538ad10 agent/src/heapstats-engines/timer.cpp --- a/agent/src/heapstats-engines/timer.cpp Thu Jul 25 00:04:53 2019 +0900 +++ b/agent/src/heapstats-engines/timer.cpp Sun Nov 10 00:26:07 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); } } diff -r b037430367a7 -r b7dba538ad10 agent/src/heapstats-engines/trapSender.cpp --- a/agent/src/heapstats-engines/trapSender.cpp Thu Jul 25 00:04:53 2019 +0900 +++ b/agent/src/heapstats-engines/trapSender.cpp Sun Nov 10 00:26:07 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) } /*! diff -r b037430367a7 -r b7dba538ad10 agent/src/heapstats-engines/util.hpp --- a/agent/src/heapstats-engines/util.hpp Thu Jul 25 00:04:53 2019 +0900 +++ b/agent/src/heapstats-engines/util.hpp Sun Nov 10 00:26:07 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 #endif +#include #include #include #include +#include /* 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.