Merge pull request #122 from corhere/err-remove-thread-state

Free thread-local OpenSSL state on thread exit

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)

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)) \

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)
+}

thread_setup.h

@@ -0,0 +1,4 @@
+#define CRYPTO_LOCK 0x01
+
+/* Used by unit tests. */
+extern volatile unsigned int go_openssl_threads_cleaned_up;

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)
+	}
+}

thread_setup_unix.c

@@ -1,13 +1,17 @@
 //go:build unix
 
 #include "goopenssl.h"
+#include "thread_setup.h"
 #include <pthread.h>
-
-#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;

thread_setup_windows.c

@@ -1,15 +1,19 @@
 //go:build windows
 
 #include "goopenssl.h"
+#include "thread_setup.h"
 
 #include <stdlib.h>
 #include <windows.h>
 
-#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,16 +22,43 @@ 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;
     int i;
     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;
 }