diff --git a/goopenssl.h b/goopenssl.h index a0e2b623..5b658ec9 100644 --- a/goopenssl.h +++ b/goopenssl.h @@ -4,6 +4,8 @@ #include "shims.h" +// Suppress warnings about unused parameters. +#define UNUSED(x) (void)(x) static inline void go_openssl_do_leak_check(void) diff --git a/shims.h b/shims.h index 4457a3e4..99656f0c 100644 --- a/shims.h +++ b/shims.h @@ -172,6 +172,7 @@ DEFINEFUNC_3_0(unsigned long, ERR_get_error_all, (const char **file, int *line, DEFINEFUNC_RENAMED_1_1(const char *, OpenSSL_version, SSLeay_version, (int type), (type)) \ DEFINEFUNC(void, OPENSSL_init, (void), ()) \ DEFINEFUNC_LEGACY_1_0(void, ERR_load_crypto_strings, (void), ()) \ +DEFINEFUNC_LEGACY_1_0(void, ERR_remove_thread_state, (const GO_CRYPTO_THREADID_PTR tid), (tid)) \ DEFINEFUNC_LEGACY_1_0(int, CRYPTO_num_locks, (void), ()) \ DEFINEFUNC_LEGACY_1_0(int, CRYPTO_THREADID_set_callback, (void (*threadid_func) (GO_CRYPTO_THREADID_PTR)), (threadid_func)) \ DEFINEFUNC_LEGACY_1_0(void, CRYPTO_THREADID_set_numeric, (GO_CRYPTO_THREADID_PTR id, unsigned long val), (id, val)) \ diff --git a/thread_setup.go b/thread_setup.go new file mode 100644 index 00000000..314bf1ef --- /dev/null +++ b/thread_setup.go @@ -0,0 +1,12 @@ +package openssl + +// Go wrappers for testing the thread setup code as _test.go files cannot import "C". + +// #include "thread_setup.h" +import "C" + +// opensslThreadsCleanedUp returns the number of times the thread-local OpenSSL +// state has been cleaned up since the process started. +func opensslThreadsCleanedUp() uint { + return uint(C.go_openssl_threads_cleaned_up) +} diff --git a/thread_setup.h b/thread_setup.h new file mode 100644 index 00000000..98d12f82 --- /dev/null +++ b/thread_setup.h @@ -0,0 +1,4 @@ +#define CRYPTO_LOCK 0x01 + +/* Used by unit tests. */ +extern volatile unsigned int go_openssl_threads_cleaned_up; diff --git a/thread_setup_test.go b/thread_setup_test.go new file mode 100644 index 00000000..57e41aaa --- /dev/null +++ b/thread_setup_test.go @@ -0,0 +1,40 @@ +package openssl + +import ( + "runtime" + "testing" + "time" +) + +func init() { + // The runtime parks the "main" thread of the process on Linux rather of + // terminating it. Lock the main thread to the initial goroutine to ensure + // that the test goroutines will always be scheduled onto non-main threads + // that can be consistently made to terminate on demand. + runtime.LockOSThread() +} + +func TestThreadCleanup(t *testing.T) { + if vMajor > 1 || vMinor > 0 { + t.Skip("explicit thread cleanup is only needed for OpenSSL 1.0.x") + } + + before := opensslThreadsCleanedUp() + done := make(chan struct{}) + go func() { + defer close(done) + // The thread this goroutine is running on will be terminated by the + // runtime when the goroutine exits. + runtime.LockOSThread() + // Checking for errors has the side effect of initializing + // the thread-local OpenSSL error queue. + _ = newOpenSSLError("") + }() + <-done + time.Sleep(100 * time.Millisecond) // Give some time for the thread to terminate. + after := opensslThreadsCleanedUp() + + if n := after - before; n != 1 { + t.Errorf("expected thread cleanup to have run once, but it ran %d times", n) + } +} diff --git a/thread_setup_unix.c b/thread_setup_unix.c index dc2df609..53ea9d03 100644 --- a/thread_setup_unix.c +++ b/thread_setup_unix.c @@ -1,13 +1,17 @@ //go:build unix #include "goopenssl.h" +#include "thread_setup.h" #include - -#define CRYPTO_LOCK 0x01 /* This array will store all of the mutexes available to OpenSSL. */ static pthread_mutex_t *mutex_buf = NULL; - + +static pthread_key_t destructor_key; + +/* Used by unit tests. */ +volatile unsigned int go_openssl_threads_cleaned_up = 0; + static void locking_function(int mode, int n, const char *file, int line) { if (mode & CRYPTO_LOCK) @@ -19,10 +23,32 @@ static void locking_function(int mode, int n, const char *file, int line) static void thread_id(GO_CRYPTO_THREADID_PTR tid) { go_openssl_CRYPTO_THREADID_set_numeric(tid, (unsigned long)pthread_self()); + + // OpenSSL fetches the current thread ID whenever it does anything with the + // per-thread error state, so this function is guaranteed to be executed at + // least once on any thread with associated error state. The thread-local + // variable needs to be set to a non-NULL value so that the destructor will + // be called when the thread exits. The actual value does not matter. + (void) pthread_setspecific(destructor_key, (void*)1); +} + +static void cleanup_thread_state(void *ignored) +{ + UNUSED(ignored); + go_openssl_ERR_remove_thread_state(NULL); + // ERR_remove_thread_state(NULL) in turn calls our registered thread_id + // callback via CRYPTO_THREADID_current(), which sets the thread-local + // variable associated with this destructor to a non-NULL value. We have to + // clear the variable ourselves to prevent pthreads from calling the + // destructor again for the same thread. + (void) pthread_setspecific(destructor_key, NULL); + go_openssl_threads_cleaned_up++; } int go_openssl_thread_setup(void) { + if (pthread_key_create(&destructor_key, cleanup_thread_state) != 0) + return 0; mutex_buf = malloc(go_openssl_CRYPTO_num_locks()*sizeof(pthread_mutex_t)); if (!mutex_buf) return 0; diff --git a/thread_setup_windows.c b/thread_setup_windows.c index 7bc66d80..93281d6c 100644 --- a/thread_setup_windows.c +++ b/thread_setup_windows.c @@ -1,15 +1,19 @@ //go:build windows #include "goopenssl.h" +#include "thread_setup.h" #include #include -#define CRYPTO_LOCK 0x01 - /* This array will store all of the mutexes available to OpenSSL. */ static HANDLE *mutex_buf = NULL; +static DWORD fls_index = FLS_OUT_OF_INDEXES; + +/* Used by unit tests. */ +volatile unsigned int go_openssl_threads_cleaned_up = 0; + static void locking_function(int mode, int n, const char *file, int line) { if (mode & CRYPTO_LOCK) @@ -18,8 +22,33 @@ static void locking_function(int mode, int n, const char *file, int line) ReleaseMutex(mutex_buf[n]); } +static void thread_id(GO_CRYPTO_THREADID_PTR tid) +{ + go_openssl_CRYPTO_THREADID_set_numeric(tid, (unsigned long)GetCurrentThreadId()); + + // OpenSSL fetches the current thread ID whenever it does anything with the + // per-thread error state, so this function is guaranteed to be executed at + // least once on any thread with associated error state. As the Win32 API + // reference documentation is unclear on whether the fiber-local storage + // slot needs to be set to trigger the destructor on thread exit, set it to + // a non-NULL value just in case. + (void) FlsSetValue(fls_index, (void*)1); + go_openssl_threads_cleaned_up++; +} + +static void cleanup_thread_state(void *ignored) +{ + UNUSED(ignored); + go_openssl_ERR_remove_thread_state(NULL); +} + int go_openssl_thread_setup(void) { + // Use the fiber-local storage API to hook a callback on thread exit. + // https://devblogs.microsoft.com/oldnewthing/20191011-00/?p=102989 + fls_index = FlsAlloc(cleanup_thread_state); + if (fls_index == FLS_OUT_OF_INDEXES) + return 0; mutex_buf = malloc(go_openssl_CRYPTO_num_locks()*sizeof(HANDLE)); if (!mutex_buf) return 0; @@ -27,7 +56,9 @@ int go_openssl_thread_setup(void) for (i = 0; i < go_openssl_CRYPTO_num_locks(); i++) mutex_buf[i] = CreateMutex(NULL, FALSE, NULL); go_openssl_CRYPTO_set_locking_callback(locking_function); - // go_openssl_CRYPTO_set_id_callback is not needed on Windows + // go_openssl_CRYPTO_set_id_callback is not strictly needed on Windows // as OpenSSL uses GetCurrentThreadId() by default. + // But we need to piggyback off the callback for our own purposes. + go_openssl_CRYPTO_THREADID_set_callback(thread_id); return 1; }