From 159dfd7c3b9ea12a4d83c99c1eaa121f4be605c3 Mon Sep 17 00:00:00 2001 From: Chris Peterson Date: Sun, 19 Feb 2012 23:11:03 -0800 Subject: [PATCH 1/3] rt: Delete Windows CRITICAL_SECTION in dtor --- src/rt/sync/lock_and_signal.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/rt/sync/lock_and_signal.cpp b/src/rt/sync/lock_and_signal.cpp index f9d40681a60a4..f3d6d43dd0a60 100644 --- a/src/rt/sync/lock_and_signal.cpp +++ b/src/rt/sync/lock_and_signal.cpp @@ -34,6 +34,7 @@ lock_and_signal::lock_and_signal() lock_and_signal::~lock_and_signal() { #if defined(__WIN32__) CloseHandle(_event); + DeleteCriticalSection(&_cs); #else CHECKED(pthread_cond_destroy(&_cond)); CHECKED(pthread_mutex_destroy(&_mutex)); From 9f492932324aded094042759eeb5824c64aa98e0 Mon Sep 17 00:00:00 2001 From: Chris Peterson Date: Sun, 19 Feb 2012 23:13:31 -0800 Subject: [PATCH 2/3] rt: Initialize Windows CRITICAL_SECTION with non-zero spin count If a CRITICAL_SECTION is not initialized with a spin count, it will default to 0, even on multi-processor systems. MSDN suggests using 4000. On single-processor systems, the spin count parameter is ignored and the critical section's spin count defaults to 0. For Windows >= Vista, extra debug info is allocated for CRITICAL_SECTIONs but not released in a timely manner. Consider using InitializeCriticalSectionEx(CRITICAL_SECTION_NO_DEBUG_INFO). --- src/rt/sync/lock_and_signal.cpp | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/rt/sync/lock_and_signal.cpp b/src/rt/sync/lock_and_signal.cpp index f3d6d43dd0a60..99861ae427967 100644 --- a/src/rt/sync/lock_and_signal.cpp +++ b/src/rt/sync/lock_and_signal.cpp @@ -19,7 +19,18 @@ lock_and_signal::lock_and_signal() : _holding_thread(INVALID_THREAD) { _event = CreateEvent(NULL, FALSE, FALSE, NULL); - InitializeCriticalSection(&_cs); + + // If a CRITICAL_SECTION is not initialized with a spin count, it will + // default to 0, even on multi-processor systems. MSDN suggests using + // 4000. On single-processor systems, the spin count parameter is ignored + // and the critical section's spin count defaults to 0. + const DWORD SPIN_COUNT = 4000; + CHECKED(!InitializeCriticalSectionAndSpinCount(&_cs, SPIN_COUNT)); + + // TODO? Consider checking GetProcAddress("InitializeCriticalSectionEx") + // so Windows >= Vista we can use CRITICAL_SECTION_NO_DEBUG_INFO to avoid + // allocating CRITICAL_SECTION debug info that is never released. See: + // http://stackoverflow.com/questions/804848/critical-sections-leaking-memory-on-vista-win2008#889853 } #else From fed81c2cfc2c16e4a2c66f25b83e34985af70f8d Mon Sep 17 00:00:00 2001 From: Chris Peterson Date: Sun, 19 Feb 2012 23:15:35 -0800 Subject: [PATCH 3/3] rt: Add some lock_and_signal assertions Assert that locks are not reentered on the same thread, unlocked by a different thread, or deleted while locked. --- src/rt/sync/lock_and_signal.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/rt/sync/lock_and_signal.cpp b/src/rt/sync/lock_and_signal.cpp index 99861ae427967..fa19b63385dd2 100644 --- a/src/rt/sync/lock_and_signal.cpp +++ b/src/rt/sync/lock_and_signal.cpp @@ -43,6 +43,7 @@ lock_and_signal::lock_and_signal() #endif lock_and_signal::~lock_and_signal() { + assert(_holding_thread == INVALID_THREAD); #if defined(__WIN32__) CloseHandle(_event); DeleteCriticalSection(&_cs); @@ -53,6 +54,7 @@ lock_and_signal::~lock_and_signal() { } void lock_and_signal::lock() { + assert(!lock_held_by_current_thread()); #if defined(__WIN32__) EnterCriticalSection(&_cs); _holding_thread = GetCurrentThreadId(); @@ -63,6 +65,7 @@ void lock_and_signal::lock() { } void lock_and_signal::unlock() { + assert(lock_held_by_current_thread()); _holding_thread = INVALID_THREAD; #if defined(__WIN32__) LeaveCriticalSection(&_cs); @@ -81,9 +84,11 @@ void lock_and_signal::wait() { LeaveCriticalSection(&_cs); WaitForSingleObject(_event, INFINITE); EnterCriticalSection(&_cs); + assert(_holding_thread == INVALID_THREAD); _holding_thread = GetCurrentThreadId(); #else CHECKED(pthread_cond_wait(&_cond, &_mutex)); + assert(_holding_thread == INVALID_THREAD); _holding_thread = pthread_self(); #endif }