From fd2d4759ff7fb6ae038daef146fb7f5ba40c5e64 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Sat, 5 Dec 2020 22:22:38 +0000 Subject: threads: git_tls_data to git_tlsdata Use a no-allocation approach to the TLS data abstraction. --- src/thread.c | 207 ++++++++++++++++-------------------------------- src/thread.h | 60 +++++++------- tests/threads/tlsdata.c | 65 +++++++++++++++ 3 files changed, 164 insertions(+), 168 deletions(-) create mode 100644 tests/threads/tlsdata.c diff --git a/src/thread.c b/src/thread.c index d9ca374e9..3171771d7 100644 --- a/src/thread.c +++ b/src/thread.c @@ -7,207 +7,134 @@ #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; + + *key = tlsdata_cnt; + tlsdata_cnt++; return 0; } -int git_tls_data__set(git_tls_data *tls, void *payload) +int git_tlsdata_set(git_tlsdata_key key, void *value) { - tls->storage = payload; + if (key < 0 || key > tlsdata_cnt) + return -1; + + tlsdata_values[key].value = value; return 0; } -void *git_tls_data__get(git_tls_data *tls) +void *git_tlsdata_get(git_tlsdata_key key) { - return tls->storage; + if (key < 0 || key > tlsdata_cnt) + return NULL; + + return tlsdata_values[key].value; } -void git_tls_data__free(git_tls_data *tls) +int git_tlsdata_dispose(git_tlsdata_key key) { - tls->free_fn(tls->storage); - git__free(tls); -} + void *value; + void (*destroy_fn)(void *) = NULL; -#elif defined(GIT_WIN32) + if (key < 0 || key > tlsdata_cnt) + return -1; -struct git_tls_data { - void GIT_CALLBACK(free_fn)(void *payload); - DWORD fls_index; -}; + value = tlsdata_values[key].value; + destroy_fn = tlsdata_values[key].destroy_fn; -struct git_tls_cell { - void GIT_CALLBACK(free_fn)(void *storage); - void *storage; -}; + tlsdata_values[key].value = NULL; + tlsdata_values[key].destroy_fn = NULL; -static void WINAPI git_tls_cell__free(void *sc) -{ - struct git_tls_cell *storage_cell = sc; - if (storage_cell == NULL) { - return; - } + if (value && destroy_fn) + destroy_fn(value); - storage_cell->free_fn(storage_cell->storage); - git__free(storage_cell); + return 0; } -int git_tls_data__init(git_tls_data **out, - void GIT_CALLBACK(free_fn)(void *payload)) +#elif defined(GIT_WIN32) + +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); + DWORD fls_index = FlsAlloc(destroy_fn); - if ((tls->fls_index = FlsAlloc(git_tls_cell__free)) == FLS_OUT_OF_INDEXES) { - git__free(tls); + if (fls_index == FLS_OUT_OF_INDEXES) return -1; - } - - tls->free_fn = free_fn; - *out = tls; + *key = fls_index; return 0; } -int git_tls_data__set(git_tls_data *tls, void *payload) +int git_tlsdata_set(git_tlsdata_key key, void *value) { - struct git_tls_cell *storage_cell; - - if (payload == NULL) { - if ((storage_cell = FlsGetValue(tls->fls_index)) != NULL) - git_tls_cell__free(storage_cell); - - if (FlsSetValue(tls->fls_index, NULL) == 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 (FlsSetValue(tls->fls_index, storage_cell) == 0) { - git__free(storage_cell); + if (!FlsSetValue(key, value)) 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 = FlsGetValue(tls->fls_index); - if (storage_cell == NULL) - return NULL; - - return storage_cell->storage; + return FlsGetValue(key); } -void git_tls_data__free(git_tls_data *tls) +int git_tlsdata_dispose(git_tlsdata_key key) { - FlsFree(tls->fls_index); - tls->free_fn = NULL; - git__free(tls); + if (!FlsFree(key)) + return -1; + + 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) +int git_tlsdata_init(git_tlsdata_key *key, void (GIT_SYSTEM_CALL *destroy_fn)(void *)) { - struct git_tls_cell *storage_cell = sc; - storage_cell->free_fn(storage_cell->storage); - git__free(storage_cell); -} - -int git_tls_data__init(git_tls_data **out, - void GIT_CALLBACK(free_fn)(void *payload)) -{ - 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 (pthread_key_create(key, destroy_fn) != 0) return -1; - } - - tls->free_fn = free_fn; - *out = tls; return 0; } -int git_tls_data__set(git_tls_data *tls, void *payload) +int git_tlsdata_set(git_tlsdata_key key, void *value) { - struct git_tls_cell *storage_cell; - - if (payload == NULL) { - if ((storage_cell = pthread_getspecific(tls->tls_key)) != NULL) - git_tls_cell__free(storage_cell); - - if (pthread_setspecific(tls->tls_key, NULL) != 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); + 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 -# error unknown threading model +# error unknown threading model #endif diff --git a/src/thread.h b/src/thread.h index 6cdf7382b..b4f869243 100644 --- a/src/thread.h +++ b/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 diff --git a/tests/threads/tlsdata.c b/tests/threads/tlsdata.c new file mode 100644 index 000000000..7c69b4444 --- /dev/null +++ b/tests/threads/tlsdata.c @@ -0,0 +1,65 @@ +#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 +} -- cgit v1.2.1