From 210bd523f6351fa35d510f08ff5044e4c27fe363 Mon Sep 17 00:00:00 2001 From: Ken Raeburn Date: Wed, 25 Jan 2006 08:05:24 +0000 Subject: [PATCH] Remove the thread-safety flag from the kdb plugin interface. Instead, 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 | 9 ++ src/lib/kdb/kdb5.c | 20 +-- src/lib/kdb/kdb5.h | 4 - src/plugins/kdb/db2/ChangeLog | 14 +++ src/plugins/kdb/db2/Makefile.in | 1 + src/plugins/kdb/db2/db2_exp.c | 213 ++++++++++++++++++++++++++++---- src/plugins/kdb/db2/kdb_db2.h | 5 - 7 files changed, 213 insertions(+), 53 deletions(-) diff --git a/src/lib/kdb/ChangeLog b/src/lib/kdb/ChangeLog index ae159d434..b6677ffee 100644 --- a/src/lib/kdb/ChangeLog +++ b/src/lib/kdb/ChangeLog @@ -1,3 +1,12 @@ +2006-01-25 Ken Raeburn + + * 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 * kdb5.c (kdb_load_library): Make dbpath_names static, to keep diff --git a/src/lib/kdb/kdb5.c b/src/lib/kdb/kdb5.c index 91f4f8ee3..156ebb65a 100644 --- a/src/lib/kdb/kdb5.c +++ b/src/lib/kdb/kdb5.c @@ -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); } diff --git a/src/lib/kdb/kdb5.h b/src/lib/kdb/kdb5.h index 93b594a52..4027849e0 100644 --- a/src/lib/kdb/kdb5.h +++ b/src/lib/kdb/kdb5.h @@ -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; diff --git a/src/plugins/kdb/db2/ChangeLog b/src/plugins/kdb/db2/ChangeLog index e88e25d73..c6461f9ad 100644 --- a/src/plugins/kdb/db2/ChangeLog +++ b/src/plugins/kdb/db2/ChangeLog @@ -1,3 +1,17 @@ +2006-01-25 Ken Raeburn + + * 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 * Makefile.in (myfulldir, RELDIR): Updated for directory rename. diff --git a/src/plugins/kdb/db2/Makefile.in b/src/plugins/kdb/db2/Makefile.in index 6935bd467..06b037ce0 100644 --- a/src/plugins/kdb/db2/Makefile.in +++ b/src/plugins/kdb/db2/Makefile.in @@ -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@) diff --git a/src/plugins/kdb/db2/db2_exp.c b/src/plugins/kdb/db2/db2_exp.c index f119de3de..29eae0566 100644 --- a/src/plugins/kdb/db2/db2_exp.c +++ b/src/plugins/kdb/db2/db2_exp.c @@ -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 }; diff --git a/src/plugins/kdb/db2/kdb_db2.h b/src/plugins/kdb/db2/kdb_db2.h index ba03ea36f..77ca60c33 100644 --- a/src/plugins/kdb/db2/kdb_db2.h +++ b/src/plugins/kdb/db2/kdb_db2.h @@ -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, -- 2.26.2