global: separate global state from thread-local state

Our "global initialization" has accumulated some debris over the years.
It was previously responsible for both running the various global
initializers (that set up various subsystems) _and_ setting up the
"global state", which is actually the thread-local state for things
like error reporting.

Separate the thread local state out into "threadstate".  Use the normal
subsystem initialization functions that we already have to set it up.
This makes both the global initialization system and the threadstate
system simpler to reason about.

src/errors.c

@@ -7,7 +7,7 @@
 
 #include "common.h"
 
-#include "global.h"
+#include "threadstate.h"
 #include "posix.h"
 #include "buffer.h"
 
@@ -22,18 +22,18 @@ static git_error g_git_oom_error = {
 
 static void set_error_from_buffer(int error_class)
 {
-	git_error *error = &GIT_GLOBAL->error_t;
-	git_buf *buf = &GIT_GLOBAL->error_buf;
+	git_error *error = &GIT_THREADSTATE->error_t;
+	git_buf *buf = &GIT_THREADSTATE->error_buf;
 
 	error->message = buf->ptr;
 	error->klass = error_class;
 
-	GIT_GLOBAL->last_error = error;
+	GIT_THREADSTATE->last_error = error;
 }
 
 static void set_error(int error_class, char *string)
 {
-	git_buf *buf = &GIT_GLOBAL->error_buf;
+	git_buf *buf = &GIT_THREADSTATE->error_buf;
 
 	git_buf_clear(buf);
 	if (string) {
@@ -46,7 +46,7 @@ static void set_error(int error_class, char *string)
 
 void git_error_set_oom(void)
 {
-	GIT_GLOBAL->last_error = &g_git_oom_error;
+	GIT_THREADSTATE->last_error = &g_git_oom_error;
 }
 
 void git_error_set(int error_class, const char *fmt, ...)
@@ -64,7 +64,7 @@ void git_error_vset(int error_class, const char *fmt, va_list ap)
 	DWORD win32_error_code = (error_class == GIT_ERROR_OS) ? GetLastError() : 0;
 #endif
 	int error_code = (error_class == GIT_ERROR_OS) ? errno : 0;
-	git_buf *buf = &GIT_GLOBAL->error_buf;
+	git_buf *buf = &GIT_THREADSTATE->error_buf;
 
 	git_buf_clear(buf);
 	if (fmt) {
@@ -97,7 +97,7 @@ void git_error_vset(int error_class, const char *fmt, va_list ap)
 
 int git_error_set_str(int error_class, const char *string)
 {
-	git_buf *buf = &GIT_GLOBAL->error_buf;
+	git_buf *buf = &GIT_THREADSTATE->error_buf;
 
 	assert(string);
 
@@ -118,9 +118,9 @@ int git_error_set_str(int error_class, const char *string)
 
 void git_error_clear(void)
 {
-	if (GIT_GLOBAL->last_error != NULL) {
+	if (GIT_THREADSTATE->last_error != NULL) {
 		set_error(0, NULL);
-		GIT_GLOBAL->last_error = NULL;
+		GIT_THREADSTATE->last_error = NULL;
 	}
 
 	errno = 0;
@@ -131,13 +131,13 @@ void git_error_clear(void)
 
 const git_error *git_error_last(void)
 {
-	return GIT_GLOBAL->last_error;
+	return GIT_THREADSTATE->last_error;
 }
 
 int git_error_state_capture(git_error_state *state, int error_code)
 {
-	git_error *error = GIT_GLOBAL->last_error;
-	git_buf *error_buf = &GIT_GLOBAL->error_buf;
+	git_error *error = GIT_THREADSTATE->last_error;
+	git_buf *error_buf = &GIT_THREADSTATE->error_buf;
 
 	memset(state, 0, sizeof(git_error_state));
 

src/global.c

@@ -8,6 +8,7 @@
 #include "global.h"
 
 #include "alloc.h"
+#include "threadstate.h"
 #include "hash.h"
 #include "sysdir.h"
 #include "filter.h"
@@ -27,6 +28,7 @@ typedef int (*git_global_init_fn)(void);
 
 static git_global_init_fn git__init_callbacks[] = {
 	git_allocator_global_init,
+	git_threadstate_global_init,
 	git_threads_global_init,
 	git_hash_global_init,
 	git_sysdir_global_init,
@@ -53,15 +55,6 @@ void git__on_shutdown(git_global_shutdown_fn callback)
 	git__shutdown_callbacks[count - 1] = callback;
 }
 
-static void git__global_state_cleanup(git_global_st *st)
-{
-	if (!st)
-		return;
-
-	git__free(st->error_t.message);
-	st->error_t.message = NULL;
-}
-
 static int init_common(void)
 {
 	size_t i;
@@ -94,32 +87,6 @@ static void shutdown_common(void)
 	}
 }
 
-/**
- * Handle the global state with TLS
- *
- * If libgit2 is built with GIT_THREADS enabled,
- * the `git_libgit2_init()` function must be called
- * before calling any other function of the library.
- *
- * This function allocates a TLS index (using pthreads
- * or the native Win32 API) to store the global state
- * on a per-thread basis.
- *
- * Any internal method that requires global state will
- * then call `git__global_state()` which returns a pointer
- * to the global state structure; this pointer is lazily
- * allocated on each thread.
- *
- * Before shutting down the library, the
- * `git_libgit2_shutdown` method must be called to free
- * the previously reserved TLS index.
- *
- * If libgit2 is built without threading support, the
- * `git__global_statestate()` call returns a pointer to a single,
- * statically allocated global state. The `git_thread_`
- * functions are not available in that case.
- */
-
 /*
  * `git_libgit2_init()` allows subsystems to perform global setup,
  * which may take place in the global scope.  An explicit memory
@@ -130,42 +97,67 @@ static void shutdown_common(void)
  */
 #if defined(GIT_THREADS) && defined(GIT_WIN32)
 
-static DWORD _fls_index;
-static volatile LONG _mutex = 0;
+/*
+ * On Win32, we use a spinlock to provide locking semantics.  This is
+ * lighter-weight than a proper critical section.
+ */
+static volatile LONG init_spinlock = 0;
 
-static void WINAPI fls_free(void *st)
+GIT_INLINE(int) init_lock(void)
 {
-	git__global_state_cleanup(st);
-	git__free(st);
+	while (InterlockedCompareExchange(&init_spinlock, 1, 0)) { Sleep(0); }
+	return 0;
 }
 
-static int synchronized_threads_init(void)
+GIT_INLINE(int) init_unlock(void)
 {
-	int error;
+	InterlockedExchange(&init_spinlock, 0);
+	return 0;
+}
 
-	if ((_fls_index = FlsAlloc(fls_free)) == FLS_OUT_OF_INDEXES)
-		return -1;
+#elif defined(GIT_THREADS) && defined(_POSIX_THREADS)
 
-	error = init_common();
+/*
+ * On POSIX, we need to use a proper mutex for locking.  We might prefer
+ * a spinlock here, too, but there's no static initializer for a
+ * pthread_spinlock_t.
+ */
+static pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER;
 
-	return error;
+GIT_INLINE(int) init_lock(void)
+{
+	return pthread_mutex_lock(&mutex) == 0 ? 0 : -1;
 }
 
+GIT_INLINE(int) init_unlock(void)
+{
+	return pthread_mutex_unlock(&mutex) == 0 ? 0 : -1;
+}
+
+#elif defined(GIT_THREADS)
+# error unknown threading model
+#else
+
+# define init_lock() 0
+# define init_unlock() 0
+
+#endif
+
 int git_libgit2_init(void)
 {
 	int ret;
 
-	/* Enter the lock */
-	while (InterlockedCompareExchange(&_mutex, 1, 0)) { Sleep(0); }
+	if (init_lock() < 0)
+		return -1;
 
 	/* Only do work on a 0 -> 1 transition of the refcount */
 	if ((ret = git_atomic_inc(&git__n_inits)) == 1) {
-		if (synchronized_threads_init() < 0)
+		if (init_common() < 0)
 			ret = -1;
 	}
 
-	/* Exit the lock */
-	InterlockedExchange(&_mutex, 0);
+	if (init_unlock() < 0)
+		return -1;
 
 	return ret;
 }
@@ -175,163 +167,16 @@ int git_libgit2_shutdown(void)
 	int ret;
 
 	/* Enter the lock */
-	while (InterlockedCompareExchange(&_mutex, 1, 0)) { Sleep(0); }
+	if (init_lock() < 0)
+		return -1;
 
 	/* Only do work on a 1 -> 0 transition of the refcount */
-	if ((ret = git_atomic_dec(&git__n_inits)) == 0) {
+	if ((ret = git_atomic_dec(&git__n_inits)) == 0)
 		shutdown_common();
 
-		FlsFree(_fls_index);
-	}
-
 	/* Exit the lock */
-	InterlockedExchange(&_mutex, 0);
-
-	return ret;
-}
-
-git_global_st *git__global_state(void)
-{
-	git_global_st *ptr;
-
-	assert(git_atomic_get(&git__n_inits) > 0);
-
-	if ((ptr = FlsGetValue(_fls_index)) != NULL)
-		return ptr;
-
-	ptr = git__calloc(1, sizeof(git_global_st));
-	if (!ptr)
-		return NULL;
-
-	if (git_buf_init(&ptr->error_buf, 0) < 0)
-		return NULL;
-
-	FlsSetValue(_fls_index, ptr);
-	return ptr;
-}
-
-#elif defined(GIT_THREADS) && defined(_POSIX_THREADS)
-
-static pthread_key_t _tls_key;
-static pthread_mutex_t _init_mutex = PTHREAD_MUTEX_INITIALIZER;
-static pthread_once_t _once_init = PTHREAD_ONCE_INIT;
-int init_error = 0;
-
-static void cb__free_status(void *st)
-{
-	git__global_state_cleanup(st);
-	git__free(st);
-}
-
-static void init_once(void)
-{
-	pthread_key_create(&_tls_key, &cb__free_status);
-	init_error = init_common();
-}
-
-int git_libgit2_init(void)
-{
-	int ret, err;
-
-	if ((err = pthread_mutex_lock(&_init_mutex)) != 0)
-		return err;
-
-	ret = git_atomic_inc(&git__n_inits);
-	err = pthread_once(&_once_init, init_once);
-	err |= pthread_mutex_unlock(&_init_mutex);
-
-	if (err || init_error)
-		return err | init_error;
-
-	return ret;
-}
-
-int git_libgit2_shutdown(void)
-{
-	void *ptr = NULL;
-	pthread_once_t new_once = PTHREAD_ONCE_INIT;
-	int error, ret;
-
-	if ((error = pthread_mutex_lock(&_init_mutex)) != 0)
-		return error;
-
-	if ((ret = git_atomic_dec(&git__n_inits)) != 0)
-		goto out;
-
-	/* Shut down any subsystems that have global state */
-	shutdown_common();
-
-	ptr = pthread_getspecific(_tls_key);
-	pthread_setspecific(_tls_key, NULL);
-
-	git__global_state_cleanup(ptr);
-	git__free(ptr);
-
-	pthread_key_delete(_tls_key);
-	_once_init = new_once;
-
-out:
-	if ((error = pthread_mutex_unlock(&_init_mutex)) != 0)
-		return error;
-
-	return ret;
-}
-
-git_global_st *git__global_state(void)
-{
-	git_global_st *ptr;
-
-	assert(git_atomic_get(&git__n_inits) > 0);
-
-	if ((ptr = pthread_getspecific(_tls_key)) != NULL)
-		return ptr;
-
-	ptr = git__calloc(1, sizeof(git_global_st));
-	if (!ptr)
-		return NULL;
-
-	if (git_buf_init(&ptr->error_buf, 0) < 0)
-		return NULL;
-
-	pthread_setspecific(_tls_key, ptr);
-	return ptr;
-}
-
-#else
-
-static git_global_st __state;
-
-int git_libgit2_init(void)
-{
-	int ret;
-
-	/* Only init subsystems the first time */
-	if ((ret = git_atomic_inc(&git__n_inits)) != 1)
-		return ret;
-
-	if ((ret = init_common()) < 0)
-		return ret;
-
-	return 1;
-}
-
-int git_libgit2_shutdown(void)
-{
-	int ret;
-
-	/* Shut down any subsystems that have global state */
-	if ((ret = git_atomic_dec(&git__n_inits)) == 0) {
-		shutdown_common();
-		git__global_state_cleanup(&__state);
-		memset(&__state, 0, sizeof(__state));
-	}
+	if (init_unlock() < 0)
+		return -1;
 
 	return ret;
 }
-
-git_global_st *git__global_state(void)
-{
-	return &__state;
-}
-
-#endif /* GIT_THREADS */

src/global.h

@@ -9,23 +9,6 @@
 
 #include "common.h"
 
-typedef struct {
-	git_error *last_error;
-	git_error error_t;
-	git_buf error_buf;
-	char oid_fmt[GIT_OID_HEXSZ+1];
-
-	/* On Windows, this is the current child thread that was started by
-	 * `git_thread_create`.  This is used to set the thread's exit code
-	 * when terminated by `git_thread_exit`.  It is unused on POSIX.
-	 */
-	git_thread *current_thread;
-} git_global_st;
-
-git_global_st *git__global_state(void);
-
-#define GIT_GLOBAL (git__global_state())
-
 typedef void (*git_global_shutdown_fn)(void);
 
 extern void git__on_shutdown(git_global_shutdown_fn callback);

src/oid.c

@@ -9,7 +9,7 @@
 
 #include "git2/oid.h"
 #include "repository.h"
-#include "global.h"
+#include "threadstate.h"
 #include <string.h>
 #include <limits.h>
 
@@ -107,7 +107,7 @@ int git_oid_pathfmt(char *str, const git_oid *oid)
 
 char *git_oid_tostr_s(const git_oid *oid)
 {
-	char *str = GIT_GLOBAL->oid_fmt;
+	char *str = GIT_THREADSTATE->oid_fmt;
 	git_oid_nfmt(str, GIT_OID_HEXSZ + 1, oid);
 	return str;
 }

src/threadstate.c

+/*
+ * Copyright (C) the libgit2 contributors. All rights reserved.
+ *
+ * This file is part of libgit2, distributed under the GNU GPL v2 with
+ * a Linking Exception. For full terms see the included COPYING file.
+ */
+
+#include "threadstate.h"
+#include "global.h"
+
+static void threadstate_dispose(git_threadstate *threadstate);
+
+/**
+ * Handle the thread-local state
+ *
+ * `git_threadstate_global_init` will be called as part
+ * of `git_libgit2_init` (which itself must be called
+ * before calling any other function in the library).
+ *
+ * This function allocates a TLS index (using pthreads
+ * or fiber-local storage in Win32) to store the per-
+ * thread state.
+ *
+ * Any internal method that requires thread-local state
+ * will then call `git_threadstate_get()` which returns a
+ * pointer to the thread-local state structure; this
+ * structure is lazily allocated on each thread.
+ *
+ * This mechanism will register a shutdown handler
+ * (`git_threadstate_global_shutdown`) which will free the
+ * TLS index.  This shutdown handler will be called by
+ * `git_libgit2_shutdown`.
+ *
+ * If libgit2 is built without threading support, the
+ * `git_threadstate_get()` call returns a pointer to a single,
+ * statically allocated global state. The `git_thread_`
+ * functions are not available in that case.
+ */
+
+#if defined(GIT_THREADS) && defined(GIT_WIN32)
+
+static DWORD fls_index;
+
+static void git_threadstate_global_shutdown(void)
+{
+	FlsFree(fls_index);
+}
+
+static void WINAPI fls_free(void *threadstate)
+{
+	threadstate_dispose(threadstate);
+	git__free(threadstate);
+}
+
+int git_threadstate_global_init(void)
+{
+	if ((fls_index = FlsAlloc(fls_free)) == FLS_OUT_OF_INDEXES)
+		return -1;
+
+	git__on_shutdown(git_threadstate_global_shutdown);
+
+	return 0;
+}
+
+git_threadstate *git_threadstate_get(void)
+{
+	git_threadstate *threadstate;
+
+	if ((threadstate = FlsGetValue(fls_index)) != NULL)
+		return threadstate;
+
+	if ((threadstate = git__calloc(1, sizeof(git_threadstate))) == NULL ||
+	    git_buf_init(&threadstate->error_buf, 0) < 0)
+		return NULL;
+
+	FlsSetValue(fls_index, threadstate);
+	return threadstate;
+}
+
+#elif defined(GIT_THREADS) && defined(_POSIX_THREADS)
+
+static pthread_key_t tls_key;
+
+static void git_threadstate_global_shutdown(void)
+{
+	git_threadstate *threadstate;
+
+	threadstate = pthread_getspecific(tls_key);
+	pthread_setspecific(tls_key, NULL);
+
+	threadstate_dispose(threadstate);
+	git__free(threadstate);
+
+	pthread_key_delete(tls_key);
+}
+
+static void tls_free(void *threadstate)
+{
+	threadstate_dispose(threadstate);
+	git__free(threadstate);
+}
+
+int git_threadstate_global_init(void)
+{
+	if (pthread_key_create(&tls_key, &tls_free) != 0)
+		return -1;
+
+	git__on_shutdown(git_threadstate_global_shutdown);
+
+	return 0;
+}
+
+git_threadstate *git_threadstate_get(void)
+{
+	git_threadstate *threadstate;
+
+	if ((threadstate = pthread_getspecific(tls_key)) != NULL)
+		return threadstate;
+
+	if ((threadstate = git__calloc(1, sizeof(git_threadstate))) == NULL ||
+	    git_buf_init(&threadstate->error_buf, 0) < 0)
+		return NULL;
+
+	pthread_setspecific(tls_key, threadstate);
+	return threadstate;
+}
+
+#elif defined(GIT_THREADS)
+# error unknown threading model
+#else
+
+static git_threadstate threadstate;
+
+static void git_threadstate_global_shutdown(void)
+{
+	threadstate_dispose(&threadstate);
+	memset(&threadstate, 0, sizeof(git_threadstate);
+}
+
+int git_threadstate_global_init(void)
+{
+	git__on_shutdown(git_threadstate_global_shutdown);
+
+	return 0;
+}
+
+git_threadstate *git_threadstate_get(void)
+{
+	return &threadstate;
+}
+
+#endif
+
+static void threadstate_dispose(git_threadstate *threadstate)
+{
+	if (!threadstate)
+		return;
+
+	git__free(threadstate->error_t.message);
+	threadstate->error_t.message = NULL;
+}

src/threadstate.h

+/*
+ * Copyright (C) the libgit2 contributors. All rights reserved.
+ *
+ * This file is part of libgit2, distributed under the GNU GPL v2 with
+ * a Linking Exception. For full terms see the included COPYING file.
+ */
+#ifndef INCLUDE_threadstate_h__
+#define INCLUDE_threadstate_h__
+
+#include "common.h"
+
+typedef struct {
+	git_error *last_error;
+	git_error error_t;
+	git_buf error_buf;
+	char oid_fmt[GIT_OID_HEXSZ+1];
+
+	/* On Windows, this is the current child thread that was started by
+	 * `git_thread_create`.  This is used to set the thread's exit code
+	 * when terminated by `git_thread_exit`.  It is unused on POSIX.
+	 */
+	git_thread *current_thread;
+} git_threadstate;
+
+extern int git_threadstate_global_init(void);
+extern git_threadstate *git_threadstate_get(void);
+
+#define GIT_THREADSTATE (git_threadstate_get())
+
+#endif

src/win32/thread.c

@@ -7,7 +7,7 @@
 
 #include "thread.h"
 
-#include "../global.h"
+#include "../tlsdata.h"
 
 #define CLEAN_THREAD_EXIT 0x6F012842
 
@@ -28,7 +28,7 @@ static DWORD WINAPI git_win32__threadproc(LPVOID lpParameter)
 	git_thread *thread = lpParameter;
 
 	/* Set the current thread for `git_thread_exit` */
-	GIT_GLOBAL->current_thread = thread;
+	GIT_TLSDATA->current_thread = thread;
 
 	thread->result = thread->proc(thread->param);
 
@@ -99,8 +99,8 @@ int git_thread_join(
 
 void git_thread_exit(void *value)
 {
-	assert(GIT_GLOBAL->current_thread);
-	GIT_GLOBAL->current_thread->result = value;
+	assert(GIT_TLSDATA->current_thread);
+	GIT_TLSDATA->current_thread->result = value;
 	ExitThread(CLEAN_THREAD_EXIT);
 }