From fb5917679dd1cc0ee50d1d88893c07cbcd82471f Mon Sep 17 00:00:00 2001 From: Philip Kelley Date: Sat, 7 Jun 2014 12:51:48 -0400 Subject: Win32: Fix object::cache::threadmania test on x64 --- src/global.c | 4 +- src/pack-objects.c | 2 +- src/thread-utils.h | 19 ++++++-- src/win32/pthread.c | 108 +++++++++++++++++++++++------------------ src/win32/pthread.h | 21 +++++--- tests/object/cache.c | 4 +- tests/threads/refdb.c | 4 +- tests/threads/thread_helpers.c | 2 +- 8 files changed, 96 insertions(+), 68 deletions(-) diff --git a/src/global.c b/src/global.c index 7da31853e..1c4a7a1a9 100644 --- a/src/global.c +++ b/src/global.c @@ -78,7 +78,7 @@ static void git__shutdown(void) static DWORD _tls_index; static volatile LONG _mutex = 0; -static int synchronized_threads_init() +static int synchronized_threads_init(void) { int error; @@ -112,7 +112,7 @@ int git_threads_init(void) return error; } -static void synchronized_threads_shutdown() +static void synchronized_threads_shutdown(void) { /* Shut down any subsystems that have global state */ git__shutdown(); diff --git a/src/pack-objects.c b/src/pack-objects.c index 3d3330ae8..0040a826b 100644 --- a/src/pack-objects.c +++ b/src/pack-objects.c @@ -1209,7 +1209,7 @@ static int ll_find_deltas(git_packbuilder *pb, git_pobject **list, git_mutex_unlock(&target->mutex); if (!sub_size) { - git_thread_join(target->thread, NULL); + git_thread_join(&target->thread, NULL); git_cond_free(&target->cond); git_mutex_free(&target->mutex); active_threads--; diff --git a/src/thread-utils.h b/src/thread-utils.h index 50d8610a3..0cc7b04ad 100644 --- a/src/thread-utils.h +++ b/src/thread-utils.h @@ -40,12 +40,23 @@ typedef git_atomic git_atomic_ssize; #ifdef GIT_THREADS +#if defined(GIT_WIN32) + +#define git_thread git_win32_thread +#define git_thread_create(thread, attr, start_routine, arg) \ + git_win32__thread_create(thread, attr, start_routine, arg) +#define git_thread_join(thread_ptr, status) \ + git_win32__thread_join(thread_ptr, status) + +#else + #define git_thread pthread_t #define git_thread_create(thread, attr, start_routine, arg) \ pthread_create(thread, attr, start_routine, arg) -#define git_thread_kill(thread) pthread_cancel(thread) -#define git_thread_exit(status) pthread_exit(status) -#define git_thread_join(id, status) pthread_join(id, status) +#define git_thread_join(thread_ptr, status) \ + pthread_join(*(thread_ptr), status) + +#endif #if defined(GIT_WIN32) #define git_thread_yield() Sleep(0) @@ -179,8 +190,6 @@ GIT_INLINE(int64_t) git_atomic64_add(git_atomic64 *a, int64_t addend) #define git_thread unsigned int #define git_thread_create(thread, attr, start_routine, arg) 0 -#define git_thread_kill(thread) (void)0 -#define git_thread_exit(status) (void)0 #define git_thread_join(id, status) (void)0 #define git_thread_yield() (void)0 diff --git a/src/win32/pthread.c b/src/win32/pthread.c index db8927471..b14b35621 100644 --- a/src/win32/pthread.c +++ b/src/win32/pthread.c @@ -8,32 +8,63 @@ #include "pthread.h" #include "../global.h" -int pthread_create( - pthread_t *GIT_RESTRICT thread, +#define CLEAN_THREAD_EXIT 0x6F012842 + +/* The thread procedure stub used to invoke the caller's procedure + * and capture the return value for later collection. Windows will + * only hold a DWORD, but we need to be able to store an entire + * void pointer. This requires the indirection. */ +static DWORD WINAPI git_win32__threadproc(LPVOID lpParameter) +{ + git_win32_thread *thread = lpParameter; + + thread->value = thread->proc(thread->value); + + return CLEAN_THREAD_EXIT; +} + +int git_win32__thread_create( + git_win32_thread *GIT_RESTRICT thread, const pthread_attr_t *GIT_RESTRICT attr, void *(*start_routine)(void*), void *GIT_RESTRICT arg) { GIT_UNUSED(attr); - *thread = CreateThread( - NULL, 0, (LPTHREAD_START_ROUTINE)start_routine, arg, 0, NULL); - return *thread ? 0 : -1; + + thread->value = arg; + thread->proc = start_routine; + thread->thread = CreateThread( + NULL, 0, git_win32__threadproc, thread, 0, NULL); + + return thread->thread ? 0 : -1; } -int pthread_join(pthread_t thread, void **value_ptr) +int git_win32__thread_join( + git_win32_thread *thread, + void **value_ptr) { - DWORD ret = WaitForSingleObject(thread, INFINITE); + DWORD exit; + + if (WaitForSingleObject(thread->thread, INFINITE) != WAIT_OBJECT_0) + return -1; + + if (!GetExitCodeThread(thread->thread, &exit)) { + CloseHandle(thread->thread); + return -1; + } - if (ret == WAIT_OBJECT_0) { - if (value_ptr != NULL) { - *value_ptr = NULL; - GetExitCodeThread(thread, (void *)value_ptr); - } - CloseHandle(thread); - return 0; + /* Check for the thread having exited uncleanly. If exit was unclean, + * then we don't have a return value to give back to the caller. */ + if (exit != CLEAN_THREAD_EXIT) { + assert(false); + thread->value = NULL; } - return -1; + if (value_ptr) + *value_ptr = thread->value; + + CloseHandle(thread->thread); + return 0; } int pthread_mutex_init( @@ -144,9 +175,6 @@ int pthread_num_processors_np(void) return n ? n : 1; } - -static HINSTANCE win32_kernel32_dll; - typedef void (WINAPI *win32_srwlock_fn)(GIT_SRWLOCK *); static win32_srwlock_fn win32_srwlock_initialize; @@ -159,7 +187,7 @@ int pthread_rwlock_init( pthread_rwlock_t *GIT_RESTRICT lock, const pthread_rwlockattr_t *GIT_RESTRICT attr) { - (void)attr; + GIT_UNUSED(attr); if (win32_srwlock_initialize) win32_srwlock_initialize(&lock->native.srwl); @@ -217,38 +245,22 @@ int pthread_rwlock_destroy(pthread_rwlock_t *lock) return 0; } - -static void win32_pthread_shutdown(void) -{ - if (win32_kernel32_dll) { - FreeLibrary(win32_kernel32_dll); - win32_kernel32_dll = NULL; - } -} - int win32_pthread_initialize(void) { - if (win32_kernel32_dll) - return 0; - - win32_kernel32_dll = LoadLibrary("Kernel32.dll"); - if (!win32_kernel32_dll) { - giterr_set(GITERR_OS, "Could not load Kernel32.dll!"); - return -1; + HMODULE hModule = GetModuleHandleW(L"kernel32"); + + if (hModule) { + win32_srwlock_initialize = (win32_srwlock_fn) + GetProcAddress(hModule, "InitializeSRWLock"); + win32_srwlock_acquire_shared = (win32_srwlock_fn) + GetProcAddress(hModule, "AcquireSRWLockShared"); + win32_srwlock_release_shared = (win32_srwlock_fn) + GetProcAddress(hModule, "ReleaseSRWLockShared"); + win32_srwlock_acquire_exclusive = (win32_srwlock_fn) + GetProcAddress(hModule, "AcquireSRWLockExclusive"); + win32_srwlock_release_exclusive = (win32_srwlock_fn) + GetProcAddress(hModule, "ReleaseSRWLockExclusive"); } - win32_srwlock_initialize = (win32_srwlock_fn) - GetProcAddress(win32_kernel32_dll, "InitializeSRWLock"); - win32_srwlock_acquire_shared = (win32_srwlock_fn) - GetProcAddress(win32_kernel32_dll, "AcquireSRWLockShared"); - win32_srwlock_release_shared = (win32_srwlock_fn) - GetProcAddress(win32_kernel32_dll, "ReleaseSRWLockShared"); - win32_srwlock_acquire_exclusive = (win32_srwlock_fn) - GetProcAddress(win32_kernel32_dll, "AcquireSRWLockExclusive"); - win32_srwlock_release_exclusive = (win32_srwlock_fn) - GetProcAddress(win32_kernel32_dll, "ReleaseSRWLockExclusive"); - - git__on_shutdown(win32_pthread_shutdown); - return 0; } diff --git a/src/win32/pthread.h b/src/win32/pthread.h index af5b121f0..a15504ed5 100644 --- a/src/win32/pthread.h +++ b/src/win32/pthread.h @@ -16,13 +16,18 @@ # define GIT_RESTRICT __restrict__ #endif +typedef struct { + HANDLE thread; + void *(*proc)(void *); + void *value; +} git_win32_thread; + typedef int pthread_mutexattr_t; typedef int pthread_condattr_t; typedef int pthread_attr_t; typedef int pthread_rwlockattr_t; typedef CRITICAL_SECTION pthread_mutex_t; -typedef HANDLE pthread_t; typedef HANDLE pthread_cond_t; typedef struct { void *Ptr; } GIT_SRWLOCK; @@ -36,13 +41,15 @@ typedef struct { #define PTHREAD_MUTEX_INITIALIZER {(void*)-1} -int pthread_create( - pthread_t *GIT_RESTRICT thread, - const pthread_attr_t *GIT_RESTRICT attr, - void *(*start_routine)(void*), - void *GIT_RESTRICT arg); +int git_win32__thread_create( + git_win32_thread *GIT_RESTRICT, + const pthread_attr_t *GIT_RESTRICT, + void *(*) (void *), + void *GIT_RESTRICT); -int pthread_join(pthread_t, void **); +int git_win32__thread_join( + git_win32_thread *, + void **); int pthread_mutex_init( pthread_mutex_t *GIT_RESTRICT mutex, diff --git a/tests/object/cache.c b/tests/object/cache.c index b927b2514..bdf12da7a 100644 --- a/tests/object/cache.c +++ b/tests/object/cache.c @@ -229,7 +229,7 @@ void test_object_cache__threadmania(void) #ifdef GIT_THREADS for (th = 0; th < THREADCOUNT; ++th) { - cl_git_pass(git_thread_join(t[th], &data)); + cl_git_pass(git_thread_join(&t[th], &data)); cl_assert_equal_i(th, ((int *)data)[0]); git__free(data); } @@ -276,7 +276,7 @@ void test_object_cache__fast_thread_rush(void) #ifdef GIT_THREADS for (th = 0; th < THREADCOUNT*2; ++th) { void *rval; - cl_git_pass(git_thread_join(t[th], &rval)); + cl_git_pass(git_thread_join(&t[th], &rval)); cl_assert_equal_i(th, *((int *)rval)); } #endif diff --git a/tests/threads/refdb.c b/tests/threads/refdb.c index c1cd29677..94a21f259 100644 --- a/tests/threads/refdb.c +++ b/tests/threads/refdb.c @@ -84,7 +84,7 @@ void test_threads_refdb__iterator(void) #ifdef GIT_THREADS for (t = 0; t < THREADS; ++t) { - cl_git_pass(git_thread_join(th[t], NULL)); + cl_git_pass(git_thread_join(&th[t], NULL)); } #endif @@ -215,7 +215,7 @@ void test_threads_refdb__edit_while_iterate(void) } for (t = 0; t < THREADS; ++t) { - cl_git_pass(git_thread_join(th[t], NULL)); + cl_git_pass(git_thread_join(&th[t], NULL)); } #endif } diff --git a/tests/threads/thread_helpers.c b/tests/threads/thread_helpers.c index 25370dddb..760a7bd33 100644 --- a/tests/threads/thread_helpers.c +++ b/tests/threads/thread_helpers.c @@ -32,7 +32,7 @@ void run_in_parallel( #ifdef GIT_THREADS for (t = 0; t < threads; ++t) - cl_git_pass(git_thread_join(th[t], NULL)); + cl_git_pass(git_thread_join(&th[t], NULL)); memset(th, 0, threads * sizeof(git_thread)); #endif -- cgit v1.2.1