The com_err library is now probably as thread-safe as it's going to get, on

UNIX; Windows still needs work....

* com_err.c: Include stdlib.h.
(com_err_hook_lock): New mutex.
(com_err_lock_hook_handle): New function.
(com_err_va, set_com_err_hook, reset_com_err_hook): Call com_err_finish_init,
and grab the lock.
* error_message.c: Don't include k5-thread.h.
(com_err_finish_init): New function.
(com_err_initialize): Initialize the new mutex.
* error_table.h: Include k5-thread.h.
(com_err_hook_lock, com_err_finish_init): Declare.

git-svn-id: svn://anonsvn.mit.edu/krb5/trunk@16394 dc483132-0cff-0310-8789-dd5450dbe970

diff --git a/src/util/et/ChangeLog b/src/util/et/ChangeLog
index 9115708fa057857270921ea5639a78a3ebc8a5fa..caf7c990c37c2865491ff5c57479e776298a4c8c 100644 (file)
--- a/src/util/et/ChangeLog
+++ b/src/util/et/ChangeLog
@@ -1,3 +1,16 @@
+2004-06-02  Ken Raeburn  <raeburn@mit.edu>
+
+       * com_err.c: Include stdlib.h.
+       (com_err_hook_lock): New mutex.
+       (com_err_lock_hook_handle): New function.
+       (com_err_va, set_com_err_hook, reset_com_err_hook): Call
+       com_err_finish_init, and grab the lock.
+       * error_message.c: Don't include k5-thread.h.
+       (com_err_finish_init): New function.
+       (com_err_initialize): Initialize the new mutex.
+       * error_table.h: Include k5-thread.h.
+       (com_err_hook_lock, com_err_finish_init): Declare.
+
 2004-05-31  Ken Raeburn  <raeburn@mit.edu>
 
        * error_message.c (terminated): New variable.

diff --git a/src/util/et/com_err.c b/src/util/et/com_err.c
index 59db4434569216388cc84c34dca8e1798a5ea9a1..e7843ecc59b3f5f60193059285174b62e68a606b 100644 (file)
--- a/src/util/et/com_err.c
+++ b/src/util/et/com_err.c
@@ -21,6 +21,7 @@
 
 #include <stdio.h>
 #include <string.h>
+#include <stdlib.h>
 
 #include "com_err.h"
 #include "error_table.h"
@@ -30,6 +31,7 @@
 #endif
 
 static /*@null@*/ et_old_error_hook_func com_err_hook = 0;
+k5_mutex_t com_err_hook_lock = K5_MUTEX_PARTIAL_INITIALIZER;
 
 static void default_com_err_proc (const char *whoami, errcode_t code,
                                  const char *fmt, va_list ap)
@@ -89,10 +91,32 @@ void KRB5_CALLCONV com_err_va(const char *whoami,
                              const char *fmt,
                              va_list ap)
 {
-       if (!com_err_hook)
-               default_com_err_proc(whoami, code, fmt, ap);
-       else
-         (com_err_hook)(whoami, code, fmt, ap);
+    int err;
+    et_old_error_hook_func p;
+
+    err = com_err_finish_init();
+    if (err)
+       goto best_try;
+    err = k5_mutex_lock(&com_err_hook_lock);
+    if (err)
+       goto best_try;
+    p = com_err_hook ? com_err_hook : default_com_err_proc;
+    (*p)(whoami, code, fmt, ap);
+    k5_mutex_unlock(&com_err_hook_lock);
+
+best_try:
+    /* Yikes.  Our library initialization failed or we couldn't lock
+       the lock we want.  We could be in trouble.  Gosh, we should
+       probably print an error message.  Oh, wait.  That's what we're
+       trying to do.  In fact, if we're losing on initialization here,
+       there's a good chance it has to do with failed initialization
+       of the caller.  */
+    if (!com_err_hook)
+       default_com_err_proc(whoami, code, fmt, ap);
+    else
+       (com_err_hook)(whoami, code, fmt, ap);
+    assert(err == 0);
+    abort();
 }
 
 
@@ -108,19 +132,37 @@ void KRB5_CALLCONV_C com_err(const char *whoami,
 }
 
 #if !(defined(_WIN32))
+/* Make a separate function because the assert invocations below
+   use the macro expansion on some platforms, which may be insanely
+   long and incomprehensible.  */
+static int com_err_lock_hook_handle(void)
+{
+    return k5_mutex_lock(&com_err_hook_lock);
+}
+
 et_old_error_hook_func set_com_err_hook (et_old_error_hook_func new_proc)
 {
-       et_old_error_hook_func x = com_err_hook;
+       et_old_error_hook_func x;
 
+       /* Broken initialization?  What can we do?  */
+       assert(com_err_finish_init() == 0);
+       assert(com_err_lock_hook_handle() == 0);
+       x = com_err_hook;
        com_err_hook = new_proc;
+       k5_mutex_unlock(&com_err_hook_lock);
        return x;
 }
 
 et_old_error_hook_func reset_com_err_hook ()
 {
-       et_old_error_hook_func x = com_err_hook;
-    
+       et_old_error_hook_func x;
+
+       /* Broken initialization?  What can we do?  */
+       assert(com_err_finish_init() == 0);
+       assert(com_err_lock_hook_handle() == 0);
+       x = com_err_hook;
        com_err_hook = NULL;
+       k5_mutex_unlock(&com_err_hook_lock);
        return x;
 }
 #endif

diff --git a/src/util/et/error_message.c b/src/util/et/error_message.c
index e785172d929886ad7c0cc5cb77d09af749bfc1cd..07c1fcd1d184f98bd252f4459694abc4b8d6f539 100644 (file)
--- a/src/util/et/error_message.c
+++ b/src/util/et/error_message.c
@@ -26,7 +26,6 @@
 #include <string.h>
 #include "com_err.h"
 #include "error_table.h"
-#include "k5-thread.h"
 #include "k5-platform.h"
 
 #if defined(_WIN32)
@@ -49,6 +48,9 @@ int com_err_initialize(void)
 {
     int err;
     err = k5_mutex_finish_init(&et_list_lock);
+    if (err)
+       return err;
+    err = k5_mutex_finish_init(&com_err_hook_lock);
     if (err)
        return err;
     err = k5_key_register(K5_KEY_COM_ERR, free);
@@ -333,3 +335,8 @@ remove_error_table(const struct error_table * et)
     k5_mutex_unlock(&et_list_lock);
     return ENOENT;
 }
+
+int com_err_finish_init()
+{
+    return CALL_INIT_FUNCTION(com_err_initialize);
+}

diff --git a/src/util/et/error_table.h b/src/util/et/error_table.h
index 41453587c4f264975dfcb8e2a94321a0bb3e33da..ea804cf00592ef6bc7eb961e2006e9d3a6306e04 100644 (file)
--- a/src/util/et/error_table.h
+++ b/src/util/et/error_table.h
@@ -31,5 +31,9 @@ extern const char *error_table_name_r (unsigned long,
                                           /*@out@*/ /*@returned@*/ char *outbuf)
      /*@modifies outbuf@*/;
 
+#include "k5-thread.h"
+extern k5_mutex_t com_err_hook_lock;
+extern int com_err_finish_init(void);
+
 #define _ET_H
 #endif