Fixes pthread leak

codebuild/bin/s2n_dynamic_load_test.c

@@ -17,6 +17,7 @@
 #include <pthread.h>
 #include <stdio.h>
 #include <stdlib.h>
+#include <limits.h>
 
 static void *s2n_load_dynamic_lib(void *ctx)
 {
@@ -63,9 +64,10 @@ int main(int argc, char *argv[])
     /* s2n-tls library can be dynamically loaded and cleaned up safely 
      *
      * We can't use any s2n test macros because this test doesn't get linked to 
-     * s2n during compile-time.
+     * s2n during compile-time. This test is in a loop to make sure that we are
+     * cleaning up pthread keys properly.
      */
-    {
+    for (size_t i = 0; i <= PTHREAD_KEYS_MAX + 1; i++) {
         pthread_t thread_id = { 0 };
         if (pthread_create(&thread_id, NULL, &s2n_load_dynamic_lib, argv[1])) {
             exit(1);

docs/DEVELOPMENT-GUIDE.md

@@ -156,6 +156,14 @@ As discussed below, s2n-tls rarely allocates resources, and so has nothing to cl
 
 `DEFER_CLEANUP(_thealloc, _thecleanup)` is a failsafe way of ensuring that resources are cleaned up, using the ` __attribute__((cleanup())` destructor mechanism available in modern C compilers.  When the variable declared in `_thealloc` goes out of scope, the cleanup function `_thecleanup` is automatically called.  This guarantees that resources will be cleaned up, no matter how the function exits.
 
+### Lifecycle of s2n memory
+s2n states publicly that every `s2n_init()` call should be paired with an `s2n_cleanup()` call, but we also attempt to do some auto-cleanup behind the scenes because we know not every s2n-user can actually follow those steps. Unfortunately, that auto-cleanup has also caused issues because it’s not very well documented and is not guaranteed to work. Here is our general philosophy behind the auto-clean behavior.
+
+For every thread that s2n functions are called in, a small amount of thread-local memory also gets initialized. This is to ensure that our random number generator will output different numbers in different threads. This memory needs to be cleaned up per thread and users can do this themselves if they call `s2n_cleanup()` per thread. But if they forget, we utilize a pthread key that calls a destructor function that cleans up our thread-local memory when the thread closes.
+
+An important thing to note is that a call to `s2n_cleanup()` usually does not fully clean up s2n. It only cleans up the thread-local memory. This is because we have an atexit handler that does fully clean up s2n at process-exit.
+The behavior is different if the atexit handler is disabled by calling `s2n_disable_atexit()`. Then s2n is actually fully cleaned up if `s2n_cleanup()` is called on the thread that called `s2n_init()`.
+
 ### Control flow and the state machine
 
 Branches can be a source of cognitive load, as they ask the reader to follow a path of thinking, while also remembering that there is another path to be explored. When branches are nested, they can often lead to impossible to grasp combinatorial explosions. s2n-tls tries to systematically reduce the number of branches used in the code in several ways.

utils/s2n_random.c

@@ -100,6 +100,8 @@ struct s2n_rand_state {
 static pthread_key_t s2n_per_thread_rand_state_key;
 /* Needed to ensure key is initialized only once */
 static pthread_once_t s2n_per_thread_rand_state_key_once = PTHREAD_ONCE_INIT;
+/* Tracks if call to pthread_key_create failed */
+static int pthread_key_create_result;
 
 static __thread struct s2n_rand_state s2n_per_thread_rand_state = {
     .cached_fork_generation_number = 0,
@@ -169,6 +171,12 @@ S2N_RESULT s2n_get_mix_entropy(struct s2n_blob *blob)
     return S2N_RESULT_OK;
 }
 
+/* Deletes pthread key at process-exit */
+static void __attribute__((destructor)) s2n_drbg_rand_state_key_cleanup(void)
+{
+    (void) pthread_key_delete(s2n_per_thread_rand_state_key);
+}
+
 static void s2n_drbg_destructor(void *_unused_argument)
 {
     (void) _unused_argument;
@@ -178,7 +186,9 @@ static void s2n_drbg_destructor(void *_unused_argument)
 
 static void s2n_drbg_make_rand_state_key(void)
 {
-    (void) pthread_key_create(&s2n_per_thread_rand_state_key, s2n_drbg_destructor);
+    /* We can't return the output of pthread_key_create due to the parameter constraints of pthread_once.
+     * Here we store the result in a global variable that will be error checked later. */
+    pthread_key_create_result = pthread_key_create(&s2n_per_thread_rand_state_key, s2n_drbg_destructor);
 }
 
 static S2N_RESULT s2n_init_drbgs(void)
@@ -191,6 +201,7 @@ static S2N_RESULT s2n_init_drbgs(void)
     RESULT_GUARD_POSIX(s2n_blob_init(&private, s2n_private_drbg, sizeof(s2n_private_drbg)));
 
     RESULT_ENSURE(pthread_once(&s2n_per_thread_rand_state_key_once, s2n_drbg_make_rand_state_key) == 0, S2N_ERR_DRBG);
+    RESULT_ENSURE_EQ(pthread_key_create_result, 0);
 
     RESULT_GUARD(s2n_drbg_instantiate(&s2n_per_thread_rand_state.public_drbg, &public, S2N_AES_128_CTR_NO_DF_PR));
     RESULT_GUARD(s2n_drbg_instantiate(&s2n_per_thread_rand_state.private_drbg, &private, S2N_AES_256_CTR_NO_DF_PR));