From 5dd59f82ccafe8866ed1e404e2a0a9c2fdc1e85a Mon Sep 17 00:00:00 2001 From: Ken Raeburn Date: Thu, 3 Jun 2004 02:21:30 +0000 Subject: [PATCH] Checkpoint notes on thread safety technique and status of various libraries * threads.txt, thread-safe.txt: New files. git-svn-id: svn://anonsvn.mit.edu/krb5/trunk@16396 dc483132-0cff-0310-8789-dd5450dbe970 --- doc/ChangeLog | 4 ++ doc/thread-safe.txt | 167 ++++++++++++++++++++++++++++++++++++++++++++ doc/threads.txt | 57 +++++++++++++++ 3 files changed, 228 insertions(+) create mode 100644 doc/thread-safe.txt create mode 100644 doc/threads.txt diff --git a/doc/ChangeLog b/doc/ChangeLog index 9ed888063..af8281fe1 100644 --- a/doc/ChangeLog +++ b/doc/ChangeLog @@ -1,3 +1,7 @@ +2004-06-02 Ken Raeburn + + * threads.txt, thread-safe.txt: New files. + 2004-02-13 Tom Yu * build.texinfo (Solaris 9): Add section describing workaround for diff --git a/doc/thread-safe.txt b/doc/thread-safe.txt new file mode 100644 index 000000000..fbff9de50 --- /dev/null +++ b/doc/thread-safe.txt @@ -0,0 +1,167 @@ +In general, it's assumed that the library initialization function (if +initialization isn't delayed) and the library finalization function +are run in some thread-safe fashion, with no other parts of the +library in question in use. (If dlopen or dlsym in one thread starts +running the initializer, and then dlopen/dlsym in another thread +returns and lets you start accessing functions or data in the library +before the initializer is finished, that really seems like a +dlopen/dlsym bug.) + +It's also assumed that if library A depends on library B, then library +B's initializer runs first, and its finalizer last, whether loading +dynamically at run time or at process startup/exit. (It appears that +AIX 4.3.3 may violate this, at least when we use gcc's +constructor/destructor attributes in shared libraries.) + +Support for freeing the heap storage allocated by a library has NOT, +in general, been written. There are hooks, but often they ignore some +of the library's local storage, mutexes, etc. + +If shared library finalization code doesn't get run at all at dlclose +time, then you'll get memory leaks. Deal with it. + +Several debugging variables that are not part of our official API are +not protected by mutexes. In general, the only way to set them is by +changing the sources and recompiling, which obviously has no run-time +thread safety issues, or by stopping the process under a debugger, +which we blithely assert is "safe enough". + +Various libraries may call assert() and abort(). This should only be +for "can't happen" cases, and indicate programming errors. In some +cases, the compiler may be able to infer that the "can't happen" cases +really can't happen, and drop the calls, but in many cases, this is +not possible. + +There are cases (e.g., in the com_err library) where errors arising +when dealing with other errors are handled by calling abort, for lack +of anything better. We should probably clean those up someday. + +Various libraries call getenv(). This is perfectly safe, as long as +nothing is calling setenv or putenv or what have you. Of course, that +severely curtails the ability to control our libraries through that +"interface". + +---------------- + +libcom_err + +Issues: + +The callback hook support (set_com_err_hook, reset_com_err_hook, and +calls to com_err and com_err_va) uses a mutex to protect the handle on +the hook function. As a side effect of this, if a callback function +is registered which pops up a window and waits for the users' +acknowledgement, then other errors cannot be reported by other threads +until after the acknowledgement. This could be fixed with +multiple-reader-one-writer type locks, but that's a bit more +complicated. + +The Windows thread safety support is unfinished. + +The string returned by error_message may be per-thread storage. It +can be passed off between threads, but it shouldn't be in use by any +thread by the time the originating thread calls error_message again. + +Error tables must no longer be in use (including pointers returned by +error_message) when the library containing them is unloaded. + +---------------- + +libprofile (and its use in libkrb5) + +Does no checks to see if it's opened multiple instances of the same +file under different names. Does not guard against trying to open a +file while another thread or process is in the process of replacing +it, or two threads trying to update a file at the same time. The +former should be pretty safe on UNIX with atomic rename, but on +Windows there's a race condition; there's a window (so to speak) where +the filename does not correspond to an actual file. + +---------------- + +libk5crypto + +Uses of the Yarrow code from the krb5 crypto interface are protected +by a single mutex. Initialization of the Yarrow state will be done +once, the first time these routines are called. Calls directly to the +Yarrow functions are not protected. + +Uses ctype macros; what happens if the locale is changed in a +multi-threaded program? + +Debug var in pbkdf2.c. + +---------------- + +libkrb5 + +(TBD) + +Uses: ctype macros + +Uses: res_search, dn_expand, getaddrinfo, getservbyname, getnameinfo + +According to current specifications, getaddrinfo should be +thread-safe; some implementations are not, and we're not attempting to +figure out which ones. + +Uses: getpwname, getpwuid -- should use _r versions if available + +Uses: gmtime, localtime -- should use _r versions + +Uses: mkstemp, mktemp -- Are these, or our uses of them, likely to be +thread-safe? + +Uses: regcomp, regexec + +Uses: sigaction + +The use of sigaction is in the code prompting for a password; we try +to catch the keyboard interrupt character being used and turn it into +an error return from that function. + +---------------- + +libgssapi_krb5 + +(TBD) + +Uses: ctype macros + +Uses: getpwuid + +Some static data. + +---------------- + +libkrb4 +libdes425 + +I don't think we're likely to bother with these. + +Part of the krb4 API requires keeping some internal storage across +calls. + +---------------- + +libgssrpc + +Skip this. We're replacing it anyways. + +---------------- + +libkadm5* +libkdb5 + +Skip these for now. We may want the KDC libraries to be thread-safe +eventually, so the KDC can take better advantage of hyperthreaded or +multiprocessor systems. + +---------------- + +libapputils +libpty +libss + +Used by single-threaded programs only (but see above re KDC). Don't +bother for now. diff --git a/doc/threads.txt b/doc/threads.txt new file mode 100644 index 000000000..200fefa7b --- /dev/null +++ b/doc/threads.txt @@ -0,0 +1,57 @@ +Thread safety in the MIT Kerberos libraries + +The return value from krb5_cc_default_name is a handle on internal +storage from the krb5_context. It is valid only until +krb5_cc_set_default_name or krb5_free_context is called. If +krb5_cc_set_default_name may be called, the calling code must ensure +that the storage returned by krb5_cc_default_name is no longer in use +by that time. + +Any use of krb5_context must be confined to one thread at a time by +the application code. + +Uses of credentials caches, replay caches, and keytabs may happen in +multiple threads simultaneously as long as none of them destroys the +object while other threads may still be using it. (Any internal data +modification in those objects will be protected by mutexes or other +means, within the krb5 library.) + + // Between these two, we should be able to do pure compile-time + // and pure run-time initialization. + // POSIX: partial initializer is PTHREAD_MUTEX_INITIALIZER, + // finish does nothing + // Windows: partial initializer is zero/empty, + // finish does the actual work + // debug: partial initializer sets one magic value, + // finish verifies, sets a new magic value + k5_mutex_t foo_mutex = K5_MUTEX_PARTIAL_INITIALIZER; + int k5_mutex_finish_init(k5_mutex_t *); + // for dynamic allocation + int k5_mutex_init(k5_mutex_t *); + // Must work for both kinds of allocation, even if it means adding + // a flag. + int k5_mutex_destroy(k5_mutex_t *); + // + // Per library, one function to finish the static mutex + // initialization. + // + // A second function called at various possible "first" entry + // points which either calls pthread_once on the first function + // (POSIX), or checks some flag set by the first function (Windows, + // debug support), and possibly returns an error. + // + // A third function for library termination calls mutex_destroy on + // each mutex for the library. + // + // + int k5_mutex_lock(k5_mutex_t *); + int k5_mutex_unlock(k5_mutex_t *); + + + k5_key_t key; + int k5_key_create(k5_key_t *, void (*destructor)(void *)); + void *k5_getspecific(k5_key_t); + int k5_setspecific(k5_key_t, const void *); + ... stuff to signal library termination ... + +See also notes in src/include/k5-thread.h. -- 2.26.2