threads: git_tls_data to git_tlsdata

Use a no-allocation approach to the TLS data abstraction.

src/thread.c

@@ -7,205 +7,132 @@
 
 #include "common.h"
 
-#ifndef GIT_THREADS
+#if !defined(GIT_THREADS)
 
-struct git_tls_data {
-	void GIT_CALLBACK(free_fn)(void *payload);
-	void *storage;
-};
+#define TLSDATA_MAX 16
 
-int git_tls_data__init(git_tls_data **out,
-	void GIT_CALLBACK(free_fn)(void *storage))
+typedef struct {
+	void *value;
+	void (GIT_SYSTEM_CALL *destroy_fn)(void *);
+} tlsdata_value;
+
+static tlsdata_value tlsdata_values[TLSDATA_MAX];
+static int tlsdata_cnt = 0;
+
+int git_tlsdata_init(git_tlsdata_key *key, void (GIT_SYSTEM_CALL *destroy_fn)(void *))
 {
-	struct git_tls_data *tls = git__malloc(sizeof(struct git_tls_data));
-	GIT_ERROR_CHECK_ALLOC(tls);
+	if (tlsdata_cnt >= TLSDATA_MAX)
+		return -1;
 
-	tls->storage = NULL;
-	tls->free_fn = free_fn;
-	*out = tls;
+	tlsdata_values[tlsdata_cnt].value = NULL;
+	tlsdata_values[tlsdata_cnt].destroy_fn = destroy_fn;
 
-	return 0;
-}
+	*key = tlsdata_cnt;
+	tlsdata_cnt++;
 
-int git_tls_data__set(git_tls_data *tls, void *payload)
-{
-	tls->storage = payload;
 	return 0;
 }
 
-void *git_tls_data__get(git_tls_data *tls)
+int git_tlsdata_set(git_tlsdata_key key, void *value)
 {
-	return tls->storage;
-}
+	if (key < 0 || key > tlsdata_cnt)
+		return -1;
 
-void git_tls_data__free(git_tls_data *tls)
-{
-	tls->free_fn(tls->storage);
-	git__free(tls);
+	tlsdata_values[key].value = value;
+	return 0;
 }
 
-#elif defined(GIT_WIN32)
-
-struct git_tls_data {
-	void GIT_CALLBACK(free_fn)(void *payload);
-	DWORD fls_index;
-};
-
-struct git_tls_cell {
-	void GIT_CALLBACK(free_fn)(void *storage);
-	void *storage;
-};
-
-static void WINAPI git_tls_cell__free(void *sc)
+void *git_tlsdata_get(git_tlsdata_key key)
 {
-  struct git_tls_cell *storage_cell = sc;
-	if (storage_cell == NULL) {
-		return;
-	}
+	if (key < 0 || key > tlsdata_cnt)
+		return NULL;
 
-	storage_cell->free_fn(storage_cell->storage);
-	git__free(storage_cell);
+	return tlsdata_values[key].value;
 }
 
-int git_tls_data__init(git_tls_data **out,
-	void GIT_CALLBACK(free_fn)(void *payload))
+int git_tlsdata_dispose(git_tlsdata_key key)
 {
-	struct git_tls_data *tls = git__malloc(sizeof(struct git_tls_data));
-	GIT_ERROR_CHECK_ALLOC(tls);
+	void *value;
+	void (*destroy_fn)(void *) = NULL;
 
-	if ((tls->fls_index = FlsAlloc(git_tls_cell__free)) == FLS_OUT_OF_INDEXES) {
-		git__free(tls);
+	if (key < 0 || key > tlsdata_cnt)
 		return -1;
-	}
-
-	tls->free_fn = free_fn;
-	*out = tls;
-
-	return 0;
-}
 
-int git_tls_data__set(git_tls_data *tls, void *payload)
-{
-	struct git_tls_cell *storage_cell;
+	value = tlsdata_values[key].value;
+	destroy_fn = tlsdata_values[key].destroy_fn;
 
-	if (payload == NULL) {
-		if ((storage_cell = FlsGetValue(tls->fls_index)) != NULL)
-			git_tls_cell__free(storage_cell);
+	tlsdata_values[key].value = NULL;
+	tlsdata_values[key].destroy_fn = NULL;
 
-		if (FlsSetValue(tls->fls_index, NULL) == 0)
-			return -1;
+	if (value && destroy_fn)
+		destroy_fn(value);
 
 	return 0;
-	}
+}
 
-	storage_cell = git__malloc(sizeof(struct git_tls_cell));
-	GIT_ERROR_CHECK_ALLOC(storage_cell);
+#elif defined(GIT_WIN32)
 
-	storage_cell->free_fn = tls->free_fn;
-	storage_cell->storage = payload;
+int git_tlsdata_init(git_tlsdata_key *key, void (GIT_SYSTEM_CALL *destroy_fn)(void *))
+{
+	DWORD fls_index = FlsAlloc(destroy_fn);
 
-	if (FlsSetValue(tls->fls_index, storage_cell) == 0) {
-		git__free(storage_cell);
+	if (fls_index == FLS_OUT_OF_INDEXES)
 		return -1;
-	}
 
+	*key = fls_index;
 	return 0;
 }
 
-void *git_tls_data__get(git_tls_data *tls)
+int git_tlsdata_set(git_tlsdata_key key, void *value)
 {
-	struct git_tls_cell *storage_cell = FlsGetValue(tls->fls_index);
-	if (storage_cell == NULL)
-		return NULL;
-
-	return storage_cell->storage;
-}
+	if (!FlsSetValue(key, value))
+		return -1;
 
-void git_tls_data__free(git_tls_data *tls)
-{
-	FlsFree(tls->fls_index);
-	tls->free_fn = NULL;
-	git__free(tls);
+	return 0;
 }
 
-#elif defined(_POSIX_THREADS)
-
-struct git_tls_data {
-	void GIT_CALLBACK(free_fn)(void *payload);
-	pthread_key_t tls_key;
-};
-
-struct git_tls_cell {
-	void GIT_CALLBACK(free_fn)(void *storage);
-	void *storage;
-};
-
-static void git_tls_cell__free(void *sc)
+void *git_tlsdata_get(git_tlsdata_key key)
 {
-  struct git_tls_cell *storage_cell = sc;
-	storage_cell->free_fn(storage_cell->storage);
-	git__free(storage_cell);
+	return FlsGetValue(key);
 }
 
-int git_tls_data__init(git_tls_data **out,
-	void GIT_CALLBACK(free_fn)(void *payload))
+int git_tlsdata_dispose(git_tlsdata_key key)
 {
-	struct git_tls_data *tls = git__malloc(sizeof(struct git_tls_data));
-	GIT_ERROR_CHECK_ALLOC(tls);
-
-	if (pthread_key_create(&tls->tls_key, git_tls_cell__free) != 0) {
-		git__free(tls);
+	if (!FlsFree(key))
 		return -1;
-	}
-
-	tls->free_fn = free_fn;
-	*out = tls;
 
 	return 0;
 }
 
-int git_tls_data__set(git_tls_data *tls, void *payload)
-{
-	struct git_tls_cell *storage_cell;
-
-	if (payload == NULL) {
-		if ((storage_cell = pthread_getspecific(tls->tls_key)) != NULL)
-			git_tls_cell__free(storage_cell);
+#elif defined(_POSIX_THREADS)
 
-		if (pthread_setspecific(tls->tls_key, NULL) != 0)
+int git_tlsdata_init(git_tlsdata_key *key, void (GIT_SYSTEM_CALL *destroy_fn)(void *))
+{
+	if (pthread_key_create(key, destroy_fn) != 0)
 		return -1;
 
 	return 0;
-	}
-
-	storage_cell = git__malloc(sizeof(struct git_tls_cell));
-	GIT_ERROR_CHECK_ALLOC(storage_cell);
-
-	storage_cell->free_fn = tls->free_fn;
-	storage_cell->storage = payload;
+}
 
-	if (pthread_setspecific(tls->tls_key, storage_cell) != 0) {
-		git__free(storage_cell);
+int git_tlsdata_set(git_tlsdata_key key, void *value)
+{
+	if (pthread_setspecific(key, value) != 0)
 		return -1;
-	}
 
 	return 0;
 }
 
-void *git_tls_data__get(git_tls_data *tls)
+void *git_tlsdata_get(git_tlsdata_key key)
 {
-	struct git_tls_cell *storage_cell = pthread_getspecific(tls->tls_key);
-	if (storage_cell == NULL)
-		return NULL;
-
-	return storage_cell->storage;
+	return pthread_getspecific(key);
 }
 
-void git_tls_data__free(git_tls_data *tls)
+int git_tlsdata_dispose(git_tlsdata_key key)
 {
-	git_tls_data__set(tls, NULL);
-	pthread_key_delete(tls->tls_key);
-	git__free(tls);
+	if (pthread_key_delete(key) != 0)
+		return -1;
+
+	return 0;
 }
 
 #else

src/thread.h

@@ -367,52 +367,56 @@ GIT_INLINE(int64_t) git_atomic64_get(git_atomic64 *a)
 
 #endif
 
-/**
- * An opaque structure for managing TLS in the library
- */
-typedef struct git_tls_data git_tls_data;
+/* Thread-local data */
+
+#if !defined(GIT_THREADS)
+# define git_tlsdata_key int
+#elif defined(GIT_WIN32)
+# define git_tlsdata_key DWORD
+#elif defined(_POSIX_THREADS)
+# define git_tlsdata_key pthread_key_t
+#else
+# error unknown threading model
+#endif
 
 /**
- * Initializes a thread local storage container.
- * This has an implementation even without GIT_THREADS
- * which just serves to encourage use of this where TLS
- * is necessary.
+ * Create a thread-local data key.  The destroy function will be
+ * called upon thread exit.  On some platforms, it may be called
+ * when all threads have deleted their keys.
  *
- * Do not call this before the allocator has been initialized.
+ * Note that the tlsdata functions do not set an error message on
+ * failure; this is because the error handling in libgit2 is itself
+ * handled by thread-local data storage.
  *
- * @param  out a pointer to store the TLS container in
- * @param  free_fn the method that should be called when
- *                 deleting something in the TLS. Will be
- *                 registered as the clean up callback for
- *                 the OS specific TLS construct.
+ * @param key the tlsdata key
+ * @param destroy_fn function pointer called upon thread exit
  * @return 0 on success, non-zero on failure
  */
-int git_tls_data__init(git_tls_data **out,
-	void GIT_CALLBACK(free_fn)(void *payload));
+int git_tlsdata_init(git_tlsdata_key *key, void (GIT_SYSTEM_CALL *destroy_fn)(void *));
 
 /**
- * Will set a thread specific value on the TLS. Passing NULL will free the
- * currently held thread specific value.
+ * Set a the thread-local value for the given key.
  *
- * @param  tls the TLS instance to store data on
- * @param  payload the pointer to store
+ * @param key the tlsdata key to store data on
+ * @param value the pointer to store
  * @return 0 on success, non-zero on failure
  */
-int git_tls_data__set(git_tls_data *tls, void *payload);
+int git_tlsdata_set(git_tlsdata_key key, void *value);
 
 /**
- * Will get the thread specific value stored in TLS.
+ * Get the thread-local value for the given key.
  *
- * @param  tls the TLS instance to retrieve data from
+ * @param key the tlsdata key to retrieve the value of
+ * @return the pointer stored with git_tlsdata_set
  */
-void *git_tls_data__get(git_tls_data *tls);
+void *git_tlsdata_get(git_tlsdata_key key);
 
 /**
- * Must call this to clean up the TLS when no longer in use.
- * The TLS pointer is unusable after a call to this.
+ * Delete the given thread-local key.
  *
- * @param tls the TLS to free
+ * @param key the tlsdata key to dispose
+ * @return 0 on success, non-zero on failure
  */
-void git_tls_data__free(git_tls_data *tls);
+int git_tlsdata_dispose(git_tlsdata_key key);
 
 #endif

tests/threads/tlsdata.c

+#include "clar_libgit2.h"
+
+#include "thread_helpers.h"
+
+void test_threads_tlsdata__can_set_and_get(void)
+{
+	git_tlsdata_key key_one, key_two, key_three;
+
+	cl_git_pass(git_tlsdata_init(&key_one, NULL));
+	cl_git_pass(git_tlsdata_init(&key_two, NULL));
+	cl_git_pass(git_tlsdata_init(&key_three, NULL));
+
+	cl_git_pass(git_tlsdata_set(key_one, (void *)(size_t)42424242));
+	cl_git_pass(git_tlsdata_set(key_two, (void *)(size_t)0xdeadbeef));
+	cl_git_pass(git_tlsdata_set(key_three, (void *)(size_t)98761234));
+
+	cl_assert_equal_sz((size_t)42424242, git_tlsdata_get(key_one));
+	cl_assert_equal_sz((size_t)0xdeadbeef, git_tlsdata_get(key_two));
+	cl_assert_equal_sz((size_t)98761234, git_tlsdata_get(key_three));
+
+	cl_git_pass(git_tlsdata_dispose(key_one));
+	cl_git_pass(git_tlsdata_dispose(key_two));
+	cl_git_pass(git_tlsdata_dispose(key_three));
+}
+
+#ifdef GIT_THREADS
+
+static void *set_and_get(void *param)
+{
+	git_tlsdata_key *tlsdata_key = (git_tlsdata_key *)param;
+	int val;
+
+	if (git_tlsdata_set(*tlsdata_key, &val) != 0 ||
+	    git_tlsdata_get(*tlsdata_key) != &val)
+		return (void *)0;
+
+	return (void *)1;
+}
+
+#endif
+
+#define THREAD_COUNT 10
+
+void test_threads_tlsdata__threads(void)
+{
+#ifdef GIT_THREADS
+	git_thread thread[THREAD_COUNT];
+	git_tlsdata_key tlsdata;
+	int i;
+
+	cl_git_pass(git_tlsdata_init(&tlsdata, NULL));
+
+	for (i = 0; i < THREAD_COUNT; i++)
+		cl_git_pass(git_thread_create(&thread[i], set_and_get, &tlsdata));
+
+	for (i = 0; i < THREAD_COUNT; i++) {
+		void *result;
+
+		cl_git_pass(git_thread_join(&thread[i], &result));
+		cl_assert_equal_sz(1, (size_t)result);
+	}
+
+	cl_git_pass(git_tlsdata_dispose(tlsdata));
+#endif
+}