Convert ERR_STATE to new multi-threading API
authorAlessandro Ghedini <alessandro@ghedini.me>
Wed, 2 Mar 2016 17:36:17 +0000 (17:36 +0000)
committerAlessandro Ghedini <alessandro@cloudflare.com>
Tue, 8 Mar 2016 14:11:09 +0000 (14:11 +0000)
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Rich Salz <rsalz@openssl.org>
crypto/err/err.c
crypto/err/err_lcl.h [deleted file]
crypto/init.c
doc/crypto/ERR_remove_state.pod
doc/crypto/err.pod
include/openssl/err.h
util/libcrypto.num

index 181882d..36ba9c7 100644 (file)
 #include <stdarg.h>
 #include <string.h>
 #include <internal/cryptlib_int.h>
+#include <internal/threads.h>
 #include <openssl/lhash.h>
 #include <openssl/crypto.h>
 #include <openssl/buffer.h>
 #include <openssl/bio.h>
 #include <openssl/err.h>
 #include <openssl/opensslconf.h>
-#include "err_lcl.h"
 
 static void err_load_strings(int lib, ERR_STRING_DATA *str);
 
@@ -231,25 +231,18 @@ static ERR_STRING_DATA ERR_str_reasons[] = {
 };
 #endif
 
+static CRYPTO_ONCE err_init = CRYPTO_ONCE_STATIC_INIT;
+static CRYPTO_THREAD_LOCAL err_thread_local;
+
 /* Predeclarations of the "err_defaults" functions */
 static LHASH_OF(ERR_STRING_DATA) *get_hash(int create, int lockit);
 static ERR_STRING_DATA *int_err_get_item(const ERR_STRING_DATA *);
-static LHASH_OF(ERR_STATE) *int_thread_get(int create, int lockit);
-static void int_thread_release(LHASH_OF(ERR_STATE) **hash);
-static ERR_STATE *int_thread_get_item(const ERR_STATE *);
-static ERR_STATE *int_thread_set_item(ERR_STATE *);
-static void int_thread_del_item(const ERR_STATE *);
 
 /*
  * The internal state
  */
 
-/* This is a struct so that REF_PRINT_COUNT works. */
-static struct refcount {
-    int references;
-} refcount = { 0 };
 static LHASH_OF(ERR_STRING_DATA) *int_error_hash = NULL;
-static LHASH_OF(ERR_STATE) *int_thread_hash = NULL;
 static int int_err_library_number = ERR_LIB_USER;
 
 static unsigned long get_error_values(int inc, int top, const char **file,
@@ -303,106 +296,6 @@ static ERR_STRING_DATA *int_err_get_item(const ERR_STRING_DATA *d)
     return p;
 }
 
-static unsigned long err_state_hash(const ERR_STATE *a)
-{
-    return CRYPTO_THREADID_hash(&a->tid) * 13;
-}
-
-static int err_state_cmp(const ERR_STATE *a, const ERR_STATE *b)
-{
-    return CRYPTO_THREADID_cmp(&a->tid, &b->tid);
-}
-
-static LHASH_OF(ERR_STATE) *int_thread_get(int create, int lockit)
-{
-    LHASH_OF(ERR_STATE) *ret = NULL;
-
-    if (lockit)
-        CRYPTO_w_lock(CRYPTO_LOCK_ERR);
-    if (!int_thread_hash && create) {
-        int_thread_hash = lh_ERR_STATE_new(err_state_hash, err_state_cmp);
-    }
-    if (int_thread_hash != NULL) {
-        refcount.references++;
-        ret = int_thread_hash;
-    }
-    if (lockit)
-        CRYPTO_w_unlock(CRYPTO_LOCK_ERR);
-    return ret;
-}
-
-static void int_thread_release(LHASH_OF(ERR_STATE) **hash)
-{
-    int i;
-
-    if (hash == NULL || *hash == NULL)
-        return;
-
-    i = CRYPTO_add(&refcount.references, -1, CRYPTO_LOCK_ERR);
-
-    REF_PRINT_COUNT(&refcount, "ERR");
-    if (i > 0)
-        return;
-    REF_ASSERT_ISNT(i < 0);
-    *hash = NULL;
-}
-
-static ERR_STATE *int_thread_get_item(const ERR_STATE *d)
-{
-    ERR_STATE *p = NULL;
-    LHASH_OF(ERR_STATE) *hash;
-
-    CRYPTO_r_lock(CRYPTO_LOCK_ERR);
-    hash = int_thread_get(0, 0);
-    if (hash)
-        p = lh_ERR_STATE_retrieve(hash, d);
-    CRYPTO_r_unlock(CRYPTO_LOCK_ERR);
-
-    int_thread_release(&hash);
-    return p;
-}
-
-static ERR_STATE *int_thread_set_item(ERR_STATE *d)
-{
-    ERR_STATE *p = NULL;
-    LHASH_OF(ERR_STATE) *hash;
-
-    CRYPTO_w_lock(CRYPTO_LOCK_ERR);
-    hash = int_thread_get(1, 0);
-    if (hash)
-        p = lh_ERR_STATE_insert(hash, d);
-    CRYPTO_w_unlock(CRYPTO_LOCK_ERR);
-
-    int_thread_release(&hash);
-    return p;
-}
-
-static void int_thread_del_item(const ERR_STATE *d)
-{
-    ERR_STATE *p = NULL;
-    LHASH_OF(ERR_STATE) *hash;
-
-    CRYPTO_w_lock(CRYPTO_LOCK_ERR);
-    hash = int_thread_get(0, 0);
-    if (hash) {
-        p = lh_ERR_STATE_delete(hash, d);
-        /* If there are no other references, and we just removed the
-         * last item, delete the int_thread_hash */
-        if (refcount.references == 1
-            && int_thread_hash
-            && lh_ERR_STATE_num_items(int_thread_hash) == 0) {
-            refcount.references = 0;
-            lh_ERR_STATE_free(int_thread_hash);
-            int_thread_hash = NULL;
-            hash = NULL;
-        }
-    }
-    CRYPTO_w_unlock(CRYPTO_LOCK_ERR);
-
-    int_thread_release(&hash);
-    ERR_STATE_free(p);
-}
-
 #ifndef OPENSSL_NO_ERR
 # define NUM_SYS_STR_REASONS 127
 # define LEN_SYS_STR_REASON 32
@@ -768,7 +661,6 @@ void ERR_error_string_n(unsigned long e, char *buf, size_t len)
     }
 }
 
-/* BAD for multi-threading: uses a local buffer if ret == NULL */
 /*
  * ERR_error_string_n should be used instead for ret != NULL as
  * ERR_error_string cannot know how large the buffer is
@@ -789,16 +681,6 @@ LHASH_OF(ERR_STRING_DATA) *ERR_get_string_table(void)
     return get_hash(0, 1);
 }
 
-LHASH_OF(ERR_STATE) *ERR_get_err_state_table(void)
-{
-    return int_thread_get(0, 1);
-}
-
-void ERR_release_err_state_table(LHASH_OF(ERR_STATE) **hash)
-{
-    int_thread_release(hash);
-}
-
 const char *ERR_lib_error_string(unsigned long e)
 {
     ERR_STRING_DATA d, *p;
@@ -838,68 +720,52 @@ const char *ERR_reason_error_string(unsigned long e)
     return ((p == NULL) ? NULL : p->string);
 }
 
-void ERR_remove_thread_state(const CRYPTO_THREADID *id)
+void ERR_remove_thread_state(void)
 {
-    ERR_STATE tmp;
+    ERR_STATE *state = ERR_get_state();
+    if (state == NULL)
+        return;
 
-    if (id)
-        CRYPTO_THREADID_cpy(&tmp.tid, id);
-    else
-        CRYPTO_THREADID_current(&tmp.tid);
-    /*
-     * thread_del_item automatically destroys the LHASH if the number of
-     * items reaches zero.
-     */
-    int_thread_del_item(&tmp);
+    CRYPTO_THREAD_set_local(&err_thread_local, NULL);
+    ERR_STATE_free(state);
 }
 
 #if OPENSSL_API_COMPAT < 0x10000000L
 void ERR_remove_state(unsigned long pid)
 {
-    ERR_remove_thread_state(NULL);
+    ERR_remove_thread_state();
 }
 #endif
 
+static void err_do_init(void)
+{
+    CRYPTO_THREAD_init_local(&err_thread_local, NULL);
+}
+
 ERR_STATE *ERR_get_state(void)
 {
-    static ERR_STATE fallback;
-    ERR_STATE *ret, tmp, *tmpp = NULL;
-    int i;
-    CRYPTO_THREADID tid;
-
-    CRYPTO_THREADID_current(&tid);
-    CRYPTO_THREADID_cpy(&tmp.tid, &tid);
-    ret = int_thread_get_item(&tmp);
-
-    /* ret == the error state, if NULL, make a new one */
-    if (ret == NULL) {
-        ret = OPENSSL_malloc(sizeof(*ret));
-        if (ret == NULL)
-            return (&fallback);
-        CRYPTO_THREADID_cpy(&ret->tid, &tid);
-        ret->top = 0;
-        ret->bottom = 0;
-        for (i = 0; i < ERR_NUM_ERRORS; i++) {
-            ret->err_data[i] = NULL;
-            ret->err_data_flags[i] = 0;
-        }
-        tmpp = int_thread_set_item(ret);
-        /* To check if insertion failed, do a get. */
-        if (int_thread_get_item(ret) != ret) {
-            ERR_STATE_free(ret); /* could not insert it */
-            return (&fallback);
+    ERR_STATE *state = NULL;
+
+    CRYPTO_THREAD_run_once(&err_init, err_do_init);
+
+    state = CRYPTO_THREAD_get_local(&err_thread_local);
+
+    if (state == NULL) {
+        state = OPENSSL_zalloc(sizeof(*state));
+        if (state == NULL)
+            return NULL;
+
+        if (!CRYPTO_THREAD_set_local(&err_thread_local, state)) {
+            ERR_STATE_free(state);
+            return NULL;
         }
-        /*
-         * If a race occurred in this function and we came second, tmpp is
-         * the first one that we just replaced.
-         */
-        ERR_STATE_free(tmpp);
 
         /* Ignore failures from these */
         OPENSSL_init_crypto(OPENSSL_INIT_LOAD_CRYPTO_STRINGS, NULL);
         ossl_init_thread_start(OPENSSL_INIT_THREAD_ERR_STATE);
     }
-    return ret;
+
+    return state;
 }
 
 int ERR_get_next_error_library(void)
diff --git a/crypto/err/err_lcl.h b/crypto/err/err_lcl.h
deleted file mode 100644 (file)
index c9d24f1..0000000
+++ /dev/null
@@ -1,2 +0,0 @@
-
-DEFINE_LHASH_OF(ERR_STATE);
index 44acd4f..b0826ee 100644 (file)
@@ -360,9 +360,9 @@ static void ossl_init_thread_stop(struct thread_local_inits_st *locals)
     if (locals->err_state) {
 #ifdef OPENSSL_INIT_DEBUG
         fprintf(stderr, "OPENSSL_INIT: ossl_init_thread_stop: "
-                        "ERR_remove_thread_state(NULL)\n");
+                        "ERR_remove_thread_state()\n");
 #endif
-        ERR_remove_thread_state(NULL);
+        ERR_remove_thread_state();
     }
 
     OPENSSL_free(locals);
index 55ded84..b011182 100644 (file)
@@ -8,7 +8,7 @@ ERR_remove_thread_state, ERR_remove_state - free a thread's error queue
 
  #include <openssl/err.h>
 
- void ERR_remove_thread_state(const CRYPTO_THREADID *tid);
+ void ERR_remove_thread_state(void);
 
 Deprecated:
 
@@ -18,17 +18,16 @@ Deprecated:
 
 =head1 DESCRIPTION
 
-ERR_remove_thread_state() frees the error queue associated with thread B<tid>.
-If B<tid> == B<NULL>, the current thread will have its error queue removed.
+ERR_remove_thread_state() frees the error queue associated with the current
+thread.
 
 Since error queue data structures are allocated automatically for new
 threads, they must be freed when threads are terminated in order to
 avoid memory leaks.
 
 ERR_remove_state is deprecated and has been replaced by
-ERR_remove_thread_state. Since threads in OpenSSL are no longer identified
-by unsigned long values any argument to this function is ignored. Calling
-ERR_remove_state is equivalent to B<ERR_remove_thread_state(NULL)>.
+ERR_remove_thread_state. Any argument to this function is ignored and
+calling ERR_remove_state is equivalent to B<ERR_remove_thread_state()>.
 
 =head1 RETURN VALUE
 
@@ -41,7 +40,6 @@ L<err(3)>
 =head1 HISTORY
 
 ERR_remove_state()
-was deprecated in OpenSSL 1.0.0 when ERR_remove_thread_state() was introduced
-and thread IDs were introduced to identify threads instead of 'unsigned long'. 
+was deprecated in OpenSSL 1.0.0 when ERR_remove_thread_state() was introduced.
 
 =cut
index 5fafbc5..6847f21 100644 (file)
@@ -22,7 +22,7 @@ err - error codes
  int ERR_GET_REASON(unsigned long e);
 
  void ERR_clear_error(void);
- void ERR_remove_thread_state(const CRYPTO_THREADID *tid);
+ void ERR_remove_thread_state(void);
 
  char *ERR_error_string(unsigned long e, char *buf);
  const char *ERR_lib_error_string(unsigned long e);
@@ -164,15 +164,14 @@ TBA more details
 
 =head1 INTERNALS
 
-The error queues are stored in a hash table with one B<ERR_STATE>
-entry for each pid. ERR_get_state() returns the current thread's
+The error queues are stored in a thread-local storage with one B<ERR_STATE>
+entry for each thread. ERR_get_state() returns the current thread's
 B<ERR_STATE>. An B<ERR_STATE> can hold up to B<ERR_NUM_ERRORS> error
 codes. When more error codes are added, the old ones are overwritten,
 on the assumption that the most recent errors are most important.
 
-Error strings are also stored in hash table. The hash tables can
-be obtained by calling ERR_get_err_state_table(void) and
-ERR_get_string_table(void) respectively.
+Error strings are also stored in a hash table that can be obtained
+by calling ERR_get_string_table(void).
 
 =head1 SEE ALSO
 
index 0b12d92..4b6c663 100644 (file)
@@ -141,7 +141,6 @@ extern "C" {
 
 # define ERR_NUM_ERRORS  16
 typedef struct err_state_st {
-    CRYPTO_THREADID tid;
     int err_flags[ERR_NUM_ERRORS];
     unsigned long err_buffer[ERR_NUM_ERRORS];
     char *err_data[ERR_NUM_ERRORS];
@@ -366,14 +365,12 @@ void ERR_load_ERR_strings(void);
 
 void ERR_free_strings(void);
 
-void ERR_remove_thread_state(const CRYPTO_THREADID *tid);
+void ERR_remove_thread_state(void);
 DEPRECATEDIN_1_0_0(void ERR_remove_state(unsigned long pid)) /* if zero we
                                                               * look it up */
 ERR_STATE *ERR_get_state(void);
 
 LHASH_OF(ERR_STRING_DATA) *ERR_get_string_table(void);
-LHASH_OF(ERR_STATE) *ERR_get_err_state_table(void);
-void ERR_release_err_state_table(LHASH_OF(ERR_STATE) **hash);
 
 int ERR_get_next_error_library(void);
 
index 28e78e4..3ae4500 100644 (file)
@@ -1047,7 +1047,7 @@ X509at_get_attr_by_OBJ                  1015      1_1_0   EXIST::FUNCTION:
 EVP_MD_CTX_copy_ex                      1016   1_1_0   EXIST::FUNCTION:
 UI_dup_error_string                     1017   1_1_0   EXIST::FUNCTION:
 lh_num_items                            1018   1_1_0   EXIST::FUNCTION:
-ERR_get_err_state_table                 1019   1_1_0   EXIST::FUNCTION:
+ERR_get_err_state_table                 1019   1_1_0   NOEXIST::FUNCTION:
 ASN1_INTEGER_cmp                        1020   1_1_0   EXIST::FUNCTION:
 X509_NAME_entry_count                   1021   1_1_0   EXIST::FUNCTION:
 UI_method_set_closer                    1022   1_1_0   EXIST::FUNCTION:
@@ -3507,7 +3507,7 @@ i2d_PBE2PARAM                           3389      1_1_0   EXIST::FUNCTION:
 EC_POINT_make_affine                    3390   1_1_0   EXIST::FUNCTION:EC
 DSA_generate_parameters                 3391   1_1_0   EXIST::FUNCTION:DEPRECATEDIN_0_9_8,DSA
 ASN1_BIT_STRING_num_asc                 3392   1_1_0   EXIST::FUNCTION:
-ERR_release_err_state_table             3393   1_1_0   EXIST::FUNCTION:
+ERR_release_err_state_table             3393   1_1_0   NOEXIST::FUNCTION:
 X509_INFO_free                          3394   1_1_0   EXIST::FUNCTION:
 d2i_PKCS8_PRIV_KEY_INFO_fp              3395   1_1_0   EXIST::FUNCTION:STDIO
 X509_OBJECT_retrieve_match              3396   1_1_0   EXIST::FUNCTION: