Remove the thread-safety flag from the kdb plugin interface. Instead,
authorKen Raeburn <raeburn@mit.edu>
Wed, 25 Jan 2006 08:05:24 +0000 (08:05 +0000)
committerKen Raeburn <raeburn@mit.edu>
Wed, 25 Jan 2006 08:05:24 +0000 (08:05 +0000)
have the kdb code assume the plugin is thread safe, and implement some
quick and dirty wrapper functions in the db2 plugin to make it use a
local mutex.

There's still some mutex code in the kdb library that should be
reviewed, and simplified or removed.

ticket: 3416
status: open

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

src/lib/kdb/ChangeLog
src/lib/kdb/kdb5.c
src/lib/kdb/kdb5.h
src/plugins/kdb/db2/ChangeLog
src/plugins/kdb/db2/Makefile.in
src/plugins/kdb/db2/db2_exp.c
src/plugins/kdb/db2/kdb_db2.h

index ae159d434a7234824c75ad8dd8051e055d0cfdf9..b6677ffee188d46c38257987d9cff864a98d459f 100644 (file)
@@ -1,3 +1,12 @@
+2006-01-25  Ken Raeburn  <raeburn@mit.edu>
+
+       * kdb5.h (struct _db_library): Delete unlocked and lock_holder
+       fields.
+       (struct _kdb_vftabl): Delete is_thread_safe field.
+       * kdb5.c (kdb_init_lib_lock, kdb_destroy_lib_lock,
+       kdb_lock_lib_lock, kdb_unlock_lib_lock): Delete references.
+       Assume the plugin is always thread-safe.
+
 2005-12-02  Ken Raeburn  <raeburn@mit.edu>
 
        * kdb5.c (kdb_load_library): Make dbpath_names static, to keep
index 91f4f8ee38baeb6ff075e145441efbacfd347540..156ebb65a5ebbb890b4509dc0c0030e425977e31 100644 (file)
@@ -84,11 +84,10 @@ kdb_init_lib_lock(db_library lib)
        return retval;
     }
 
-    lib->lock_holder = pthread_self();
     lib->excl = 0;
     lib->recursive_cnt = 0;
 
-    return pthread_cond_init(&lib->unlocked, NULL);
+    return 0;
 }
 
 static int
@@ -99,7 +98,7 @@ kdb_destroy_lib_lock(db_library lib)
        return retval;
     }
 
-    return pthread_cond_destroy(&lib->unlocked);
+    return 0;
 }
 
 static int
@@ -108,20 +107,10 @@ kdb_lock_lib_lock(db_library lib, krb5_boolean exclusive)
     /* Since, handle locked by one thread should not allow another
        thread to continue.  */
     krb5_error_code retval = 0;
-    pthread_t myid = pthread_self();
 
     if ((retval = pthread_mutex_lock(&lib->lib_lock)))
        return retval;
 
-    while ((exclusive && (lib->excl || lib->recursive_cnt)) ||
-          (!pthread_equal(lib->lock_holder, myid)
-           && !lib->vftabl.is_thread_safe && lib->recursive_cnt)) {
-       /* Exclusive lock held or some one using lock when exclusive
-          is requested or library not-re-entrant.  */
-       if ((retval = pthread_cond_wait(&lib->unlocked, &lib->lib_lock)))
-           return retval;
-    }
-
     /* exclusive lock and recursive_cnt allow a thread to lock even it
        already holds a lock */
     if (exclusive)
@@ -129,8 +118,6 @@ kdb_lock_lib_lock(db_library lib, krb5_boolean exclusive)
 
     lib->recursive_cnt++;
 
-    lib->lock_holder = myid;
-
     return pthread_mutex_unlock(&lib->lib_lock);
 }
 
@@ -146,9 +133,6 @@ kdb_unlock_lib_lock(db_library lib, krb5_boolean exclusive)
     if (exclusive)
        lib->excl--;
 
-    if ((retval = pthread_cond_broadcast(&lib->unlocked)))
-       return retval;
-
     return pthread_mutex_unlock(&lib->lib_lock);
 }
 
index 93b594a52af422c08a3750486accc30bf111c983..4027849e002e67e918f1b7609f056f1963417307 100644 (file)
@@ -49,8 +49,6 @@ typedef struct _kdb_vftabl{
     short int maj_ver;
     short int min_ver;
 
-    short int is_thread_safe;
-
     krb5_error_code (*init_library)(krb5_set_err_func_t);
     krb5_error_code (*fini_library)();
     krb5_error_code (*init_module) ( krb5_context kcontext,
@@ -200,9 +198,7 @@ typedef struct _db_library {
     int reference_cnt;
 #ifdef HAVE_PTHREAD_H
     pthread_mutex_t lib_lock;
-    pthread_cond_t unlocked; /*  To check whether some one has called db_unlock */
     int recursive_cnt;               /* this is used as lock to help recursive locking */
-    pthread_t lock_holder;
     int excl;
 #endif
     void *dl_handle;
index e88e25d732d4feb9ec6a4bddabb9a2d534a96573..c6461f9ad8fc1ec494e070477c03fc7e4587882d 100644 (file)
@@ -1,3 +1,17 @@
+2006-01-25  Ken Raeburn  <raeburn@mit.edu>
+
+       * Makefile.in (DEFINES): New variable; define macro PLUGIN.
+       * db2_exp.c (krb5_db2_mutex): New variable.
+       (wrap_*): Lots of new functions; lock the mutex, call the real
+       function, unlock the mutex.
+       (WRAP, WRAP_K, WRAP_VOID): Quick and dirty macros for implementing
+       the above.
+       (hack_init, hack_cleanup): New functions.  Call the regular init
+       or cleanup function, but also deal with the new mutex.
+       (krb5_db_vftabl_db2): Use the hack/wrap functions.
+
+       * kdb_db2.h (krb5_db2_get_policy): Delete duplicate declaration.
+
 2005-12-16  Ken Raeburn  <raeburn@mit.edu>
 
        * Makefile.in (myfulldir, RELDIR): Updated for directory rename.
index 6935bd4679a4dc35b3914d7dd9b49b1555955646..06b037ce0e5013eb5a4faa6e7082f3e68f3c58c8 100644 (file)
@@ -8,6 +8,7 @@ PROG_LIBPATH=-L$(TOPLIBD)
 PROG_RPATH=$(KRB5_LIBDIR)
 
 LOCALINCLUDES = -I../../../lib/kdb -I$(srcdir)/../../../lib/kdb
+DEFINES = -DPLUGIN
 
 DB_VERSION     = @DB_VERSION@
 DB_DEPS                = $(DB_DEPS-@DB_HEADER_VERSION@)
index f119de3de94491932f6bffa02d9c27efbbac4de2..29eae056694d9f3037661fef619f521139730d90 100644 (file)
@@ -25,6 +25,168 @@ static char *_csrc = "@(#) %filespec: db2_exp.c~5 %  (%full_filespec: db2_exp.c~
 #include "kdb_xdr.h"
 #include "policy_db.h"
 
+/* Quick and dirty wrapper functions to provide for thread safety
+   within the plugin, instead of making the kdb5 library do it.  Eventually
+   these should be integrated into the real functions.
+
+   Some of the functions wrapped here are also called directly from
+   within this library (e.g., create calls open), so simply dropping
+   locking code into the top and bottom of each referenced function
+   won't do.  (We aren't doing recursive locks, currently.)  */
+
+static k5_mutex_t *krb5_db2_mutex;
+
+#define WRAP(NAME,TYPE,ARGLIST,ARGNAMES,ERROR_RESULT)  \
+       static TYPE wrap_##NAME ARGLIST                 \
+       {                                               \
+           TYPE result;                                \
+           int code = k5_mutex_lock (krb5_db2_mutex);  \
+           if (code) { return ERROR_RESULT; }          \
+           result = NAME ARGNAMES;                     \
+           k5_mutex_unlock (krb5_db2_mutex);           \
+           return result;                              \
+       }                                               \
+       /* hack: decl to allow a following ";" */       \
+       static TYPE wrap_##NAME ()
+
+/* Two special cases: void (can't assign result), and krb5_error_code
+   (return error from locking code).  */
+
+#define WRAP_VOID(NAME,ARGLIST,ARGNAMES)               \
+       static void wrap_##NAME ARGLIST                 \
+       {                                               \
+           int code = k5_mutex_lock (krb5_db2_mutex);  \
+           if (code) { return; }                       \
+           NAME ARGNAMES;                              \
+           k5_mutex_unlock (krb5_db2_mutex);           \
+       }                                               \
+       /* hack: decl to allow a following ";" */       \
+       static void wrap_##NAME ()
+
+#define WRAP_K(NAME,ARGLIST,ARGNAMES)                  \
+       WRAP(NAME,krb5_error_code,ARGLIST,ARGNAMES,code)
+
+WRAP_K (krb5_db2_open,
+       ( krb5_context kcontext,
+         char *conf_section,
+         char **db_args,
+         int mode ),
+       (kcontext, conf_section, db_args, mode));
+WRAP_K (krb5_db2_db_fini, (krb5_context ctx), (ctx));
+WRAP_K (krb5_db2_create,
+       ( krb5_context kcontext, char *conf_section, char **db_args ),
+       (kcontext, conf_section, db_args));
+WRAP_K (krb5_db2_destroy,
+       ( krb5_context kcontext, char *conf_section, char **db_args ),
+       (kcontext, conf_section, db_args));
+WRAP_K (krb5_db2_db_get_age,
+       (krb5_context ctx,
+                  char *s,
+        time_t *t),
+       (ctx, s, t));
+WRAP_K (krb5_db2_db_set_option,
+       ( krb5_context kcontext, 
+         int option, 
+         void *value ),
+       (kcontext, option, value));
+
+WRAP_K (krb5_db2_db_lock,
+       ( krb5_context    context,
+         int             in_mode),
+       (context, in_mode));
+WRAP_K (krb5_db2_db_unlock, (krb5_context ctx), (ctx));
+
+WRAP_K (krb5_db2_db_get_principal,
+       (krb5_context ctx,
+                  krb5_const_principal p,
+                  krb5_db_entry *d,
+                  int * i,
+        krb5_boolean *b),
+       (ctx, p, d, i, b));
+WRAP_K (krb5_db2_db_free_principal,
+       (krb5_context ctx,
+                  krb5_db_entry *d,
+        int i),
+       (ctx, d, i));
+WRAP_K (krb5_db2_db_put_principal,
+       (krb5_context ctx,
+        krb5_db_entry *d,
+        int *i,
+        char **db_args),
+       (ctx, d, i, db_args));
+WRAP_K (krb5_db2_db_delete_principal,
+       (krb5_context context,
+        krb5_const_principal searchfor,
+        int *nentries),
+       (context, searchfor, nentries));
+
+WRAP_K (krb5_db2_db_iterate,
+       (krb5_context ctx, char *s,
+        krb5_error_code (*f) (krb5_pointer,
+                             krb5_db_entry *),
+        krb5_pointer p),
+       (ctx, s, f, p));
+
+WRAP_K (krb5_db2_create_policy,
+       (krb5_context context, osa_policy_ent_t entry),
+       (context, entry));
+WRAP_K (krb5_db2_get_policy,
+       ( krb5_context kcontext,
+         char *name,
+         osa_policy_ent_t *policy,
+         int *cnt),
+       (kcontext, name, policy, cnt));
+WRAP_K (krb5_db2_put_policy,
+       ( krb5_context kcontext, osa_policy_ent_t policy ),
+       (kcontext, policy));
+WRAP_K (krb5_db2_iter_policy,
+       ( krb5_context kcontext,
+         char *match_entry,
+         osa_adb_iter_policy_func func,
+         void *data ),
+       (kcontext, match_entry, func, data));
+WRAP_K (krb5_db2_delete_policy,
+       ( krb5_context kcontext, char *policy ),
+       (kcontext, policy));
+WRAP_VOID (krb5_db2_free_policy,
+          ( krb5_context kcontext, osa_policy_ent_t entry ),
+          (kcontext, entry));
+
+WRAP (krb5_db2_alloc, void *,
+      ( krb5_context kcontext,  
+       void *ptr, 
+       size_t size ),
+      (kcontext, ptr, size), NULL);
+WRAP_VOID (krb5_db2_free,
+          ( krb5_context kcontext, void *ptr ),
+          (kcontext, ptr));
+
+WRAP_K (krb5_db2_set_master_key_ext,
+       ( krb5_context kcontext, char *pwd, krb5_keyblock *key),
+       (kcontext, pwd, key));
+WRAP_K (krb5_db2_db_get_mkey,
+       ( krb5_context context, krb5_keyblock **key),
+       (context, key));
+
+static krb5_error_code
+hack_init (krb5_set_err_func_t f)
+{
+    krb5_error_code c;
+    c = krb5int_mutex_alloc (&krb5_db2_mutex);
+    if (c)
+       return c;
+    return krb5_db2_lib_init (f);
+}
+
+static krb5_error_code
+hack_cleanup (void)
+{
+    krb5int_mutex_free (krb5_db2_mutex);
+    krb5_db2_mutex = NULL;
+    return krb5_db2_lib_cleanup();
+}
+
+
 /*
  *      Exposed API
  */
@@ -32,33 +194,32 @@ static char *_csrc = "@(#) %filespec: db2_exp.c~5 %  (%full_filespec: db2_exp.c~
 kdb_vftabl krb5_db_vftabl_db2 = {
   1,                                      /* major version number 1 */
   0,                                      /* minor version number 0 */
-  0,                                      /* TBD. Not sure whether thread safe. For now, its not */
-  /* init_library */                          krb5_db2_lib_init,
-  /* fini_library */                          krb5_db2_lib_cleanup,
-  /* init_module */                           krb5_db2_open,
-  /* fini_module */                           krb5_db2_db_fini,
-  /* db_create */                             krb5_db2_create,
-  /* db_destroy */                            krb5_db2_destroy,
-  /* db_get_age */                             krb5_db2_db_get_age,
-  /* db_set_option */                         krb5_db2_db_set_option,
-  /* db_lock */                                       krb5_db2_db_lock,
-  /* db_unlock */                             krb5_db2_db_unlock,
-  /* db_get_principal */                      krb5_db2_db_get_principal,
-  /* db_free_principal */                     krb5_db2_db_free_principal,
-  /* db_put_principal */                      krb5_db2_db_put_principal,
-  /* db_delete_principal */                   krb5_db2_db_delete_principal,
-  /* db_iterate */                            krb5_db2_db_iterate,
-  /* db_create_policy */                       krb5_db2_create_policy,
-  /* db_get_policy */                          krb5_db2_get_policy,
-  /* db_put_policy */                          krb5_db2_put_policy,
-  /* db_iter_policy */                         krb5_db2_iter_policy,
-  /* db_delete_policy */                       krb5_db2_delete_policy,
-  /* db_free_policy */                         krb5_db2_free_policy,
+  /* init_library */                          hack_init,
+  /* fini_library */                          hack_cleanup,
+  /* init_module */                           wrap_krb5_db2_open,
+  /* fini_module */                           wrap_krb5_db2_db_fini,
+  /* db_create */                             wrap_krb5_db2_create,
+  /* db_destroy */                            wrap_krb5_db2_destroy,
+  /* db_get_age */                             wrap_krb5_db2_db_get_age,
+  /* db_set_option */                         wrap_krb5_db2_db_set_option,
+  /* db_lock */                                       wrap_krb5_db2_db_lock,
+  /* db_unlock */                             wrap_krb5_db2_db_unlock,
+  /* db_get_principal */                      wrap_krb5_db2_db_get_principal,
+  /* db_free_principal */                     wrap_krb5_db2_db_free_principal,
+  /* db_put_principal */                      wrap_krb5_db2_db_put_principal,
+  /* db_delete_principal */                   wrap_krb5_db2_db_delete_principal,
+  /* db_iterate */                            wrap_krb5_db2_db_iterate,
+  /* db_create_policy */                       wrap_krb5_db2_create_policy,
+  /* db_get_policy */                          wrap_krb5_db2_get_policy,
+  /* db_put_policy */                          wrap_krb5_db2_put_policy,
+  /* db_iter_policy */                         wrap_krb5_db2_iter_policy,
+  /* db_delete_policy */                       wrap_krb5_db2_delete_policy,
+  /* db_free_policy */                         wrap_krb5_db2_free_policy,
   /* db_supported_realms */                   NULL,
   /* db_free_supported_realms */              NULL,
   /* errcode_2_string */                       NULL,
-  /* db_alloc */                               krb5_db2_alloc,
-  /* db_free */                                krb5_db2_free,
-  /* set_master_key */                        krb5_db2_set_master_key_ext,
-  /* get_master_key */                        krb5_db2_db_get_mkey
+  /* db_alloc */                               wrap_krb5_db2_alloc,
+  /* db_free */                                wrap_krb5_db2_free,
+  /* set_master_key */                        wrap_krb5_db2_set_master_key_ext,
+  /* get_master_key */                        wrap_krb5_db2_db_get_mkey
 };
index ba03ea36f3247c4bdd976f4125c0709e6d401795..77ca60c3348d0d875eecaada036b7c24070f3312 100644 (file)
@@ -185,11 +185,6 @@ krb5_db2_free( krb5_context kcontext,
 krb5_error_code
 krb5_db2_create_policy(krb5_context context, osa_policy_ent_t entry);
 
-krb5_error_code krb5_db2_get_policy ( krb5_context kcontext,
-                                     char *name,
-                                     osa_policy_ent_t *policy,
-                                     int *cnt);
-
 krb5_error_code krb5_db2_get_policy ( krb5_context kcontext,
                                      char *name,
                                      osa_policy_ent_t *policy,