From ac15ce2da84080df06d26556c641c85bca02385f Mon Sep 17 00:00:00 2001 From: Armin Novak Date: Tue, 28 Apr 2015 08:55:26 +0200 Subject: [PATCH 1/4] Added mutex debug flag. When mutex debugging is enabled now a stack trace is logged, if a mutex is locked on destruction. --- cmake/ConfigOptions.cmake | 1 + config.h.in | 1 + winpr/libwinpr/synch/mutex.c | 28 ++++++++++++++++++++++++++++ 3 files changed, 30 insertions(+) diff --git a/cmake/ConfigOptions.cmake b/cmake/ConfigOptions.cmake index 282659e01..b51a46b35 100644 --- a/cmake/ConfigOptions.cmake +++ b/cmake/ConfigOptions.cmake @@ -115,6 +115,7 @@ option(WITH_DEBUG_SND "Print rdpsnd debug messages" ${DEFAULT_DEBUG_OPTION}) option(WITH_DEBUG_SVC "Print static virtual channel debug messages." ${DEFAULT_DEBUG_OPTION}) option(WITH_DEBUG_TRANSPORT "Print transport debug messages." ${DEFAULT_DEBUG_OPTION}) option(WITH_DEBUG_THREADS "Print thread debug messages, enables handle dump" ${DEFAULT_DEBUG_OPTION}) +option(WITH_DEBUG_MUTEX "Print mutex debug messages" ${DEFAULT_DEBUG_OPTION}) option(WITH_DEBUG_TIMEZONE "Print timezone debug messages." ${DEFAULT_DEBUG_OPTION}) option(WITH_DEBUG_WND "Print window order debug messages" ${DEFAULT_DEBUG_OPTION}) option(WITH_DEBUG_X11_CLIPRDR "Print X11 clipboard redirection debug messages" ${DEFAULT_DEBUG_OPTION}) diff --git a/config.h.in b/config.h.in index 82c75ee9a..807148d4c 100644 --- a/config.h.in +++ b/config.h.in @@ -88,6 +88,7 @@ #cmakedefine WITH_DEBUG_RDPEI #cmakedefine WITH_DEBUG_TIMEZONE #cmakedefine WITH_DEBUG_THREADS +#cmakedefine WITH_DEBUG_MUTEX #cmakedefine WITH_DEBUG_TRANSPORT #cmakedefine WITH_DEBUG_WND #cmakedefine WITH_DEBUG_X11 diff --git a/winpr/libwinpr/synch/mutex.c b/winpr/libwinpr/synch/mutex.c index 50d950835..37c54ba00 100644 --- a/winpr/libwinpr/synch/mutex.c +++ b/winpr/libwinpr/synch/mutex.c @@ -22,12 +22,18 @@ #endif #include +#include +#include #include "synch.h" #ifndef _WIN32 #include "../handle/handle.h" + +#include "../log.h" +#define TAG WINPR_TAG("sync.mutex") + static BOOL MutexCloseHandle(HANDLE handle); static BOOL MutexIsHandled(HANDLE handle) @@ -61,6 +67,28 @@ BOOL MutexCloseHandle(HANDLE handle) if (!MutexIsHandled(handle)) return FALSE; +#if defined(WITH_DEBUG_MUTEX) + if (pthread_mutex_trylock(&mutex->mutex)) + { + size_t used = 0, i; + void* stack = winpr_backtrace(20); + char **msg = NULL; + + if (stack) + msg = winpr_backtrace_symbols(stack, &used); + + if (msg) + { + for(i=0; imutex); +#endif + if (!pthread_mutex_destroy(&mutex->mutex)) return FALSE; From fe27913859374435c41d33d834edf0b1133bcdba Mon Sep 17 00:00:00 2001 From: Armin Novak Date: Tue, 28 Apr 2015 09:42:46 +0200 Subject: [PATCH 2/4] Ensuring mutex is unlocked on destroy. --- winpr/libwinpr/synch/mutex.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/winpr/libwinpr/synch/mutex.c b/winpr/libwinpr/synch/mutex.c index 37c54ba00..ca12a1942 100644 --- a/winpr/libwinpr/synch/mutex.c +++ b/winpr/libwinpr/synch/mutex.c @@ -67,9 +67,9 @@ BOOL MutexCloseHandle(HANDLE handle) if (!MutexIsHandled(handle)) return FALSE; -#if defined(WITH_DEBUG_MUTEX) if (pthread_mutex_trylock(&mutex->mutex)) { +#if defined(WITH_DEBUG_MUTEX) size_t used = 0, i; void* stack = winpr_backtrace(20); char **msg = NULL; @@ -84,10 +84,11 @@ BOOL MutexCloseHandle(HANDLE handle) } free (msg); winpr_backtrace_free(stack); - } - else - pthread_mutex_unlock(&mutex->mutex); #endif + } + + if (pthread_mutex_unlock(&mutex->mutex)) + return FALSE; if (!pthread_mutex_destroy(&mutex->mutex)) return FALSE; From 07ee19820333dc60d82c39339d958aa8417c7976 Mon Sep 17 00:00:00 2001 From: Armin Novak Date: Tue, 28 Apr 2015 17:29:07 +0200 Subject: [PATCH 3/4] Better error handling on mutex destroy. --- winpr/libwinpr/synch/mutex.c | 50 +++++++++++++++++++++++++----------- 1 file changed, 35 insertions(+), 15 deletions(-) diff --git a/winpr/libwinpr/synch/mutex.c b/winpr/libwinpr/synch/mutex.c index ca12a1942..f5540ab1c 100644 --- a/winpr/libwinpr/synch/mutex.c +++ b/winpr/libwinpr/synch/mutex.c @@ -29,6 +29,8 @@ #ifndef _WIN32 +#include + #include "../handle/handle.h" #include "../log.h" @@ -63,35 +65,53 @@ static int MutexGetFd(HANDLE handle) BOOL MutexCloseHandle(HANDLE handle) { WINPR_MUTEX* mutex = (WINPR_MUTEX*) handle; + int rc; if (!MutexIsHandled(handle)) return FALSE; - if (pthread_mutex_trylock(&mutex->mutex)) + rc = pthread_mutex_trylock(&mutex->mutex); + switch(rc) { + /* If we already own the mutex consider it a success. */ + case EDEADLK: + break; + default: #if defined(WITH_DEBUG_MUTEX) - size_t used = 0, i; - void* stack = winpr_backtrace(20); - char **msg = NULL; - - if (stack) - msg = winpr_backtrace_symbols(stack, &used); - - if (msg) { - for(i=0; imutex)) + rc = pthread_mutex_unlock(&mutex->mutex); + if (rc != 0) + { + WLog_ERR(TAG, "pthread_mutex_unlock failed with %s [%d]", strerror(rc), rc); return FALSE; + } - if (!pthread_mutex_destroy(&mutex->mutex)) + rc = pthread_mutex_destroy(&mutex->mutex); + if (rc != 0) + { + WLog_ERR(TAG, "pthread_mutex_destroy failed with %s [%d]", strerror(rc), rc); return FALSE; + } free(handle); From b7fccafb9455380aa28e0ce8938da2fc8d482006 Mon Sep 17 00:00:00 2001 From: Armin Novak Date: Wed, 29 Apr 2015 08:48:07 +0200 Subject: [PATCH 4/4] Added EBUSY to success states. --- winpr/libwinpr/synch/mutex.c | 1 + 1 file changed, 1 insertion(+) diff --git a/winpr/libwinpr/synch/mutex.c b/winpr/libwinpr/synch/mutex.c index f5540ab1c..8591d9b6b 100644 --- a/winpr/libwinpr/synch/mutex.c +++ b/winpr/libwinpr/synch/mutex.c @@ -75,6 +75,7 @@ BOOL MutexCloseHandle(HANDLE handle) { /* If we already own the mutex consider it a success. */ case EDEADLK: + case EBUSY: break; default: #if defined(WITH_DEBUG_MUTEX)