Re: crypt mutex

[Kevin Ryde <user42@zip.com.au>]

[-- Attachment #1: Type: text/plain, Size: 1128 bytes --]

It took a while but I made the change below to protect crypt,
introducing a new scm_i_misc_mutex intended for miscellaneous uses,
like this one, which don't rate highly enough to deserve their own
personal mutex.

        * threads.c, threads.h (scm_i_misc_mutex): New SCM_GLOBAL_MUTEX.
        * posix.c (scm_crypt): Use it to protect static data in crypt().

Thread trouble in the old code can be provoked fairly easily with
something like the following.  It's not the sort of thing that can be
added to a test suite, but at least it shows the change has an effect
:-).


(use-modules (ice-9 threads))

(define k1 "blahblah")
(define s1 "99")
(define e1 (crypt k1 s1))

(define k2 "foobar")
(define s2 "77")
(define e2 (crypt k2 s2))

(begin-thread
 (while #t
   (display "-")
   (let ((e (crypt k1 s1)))
     (if (not (string=? e1 e))
         (begin
           (format #t "oops, wrong e1, got ~s want ~s\n" e e1)
           (exit 1))))))

(while #t
  (display ".")
  (let ((e (crypt k2 s2)))
    (if (not (string=? e2 e))
        (begin
          (format #t "oops, wrong e2, got ~s want ~s\n" e e2)
          (exit 1)))))



[-- Attachment #2: posix.c.crypt.diff --]
[-- Type: text/plain, Size: 2167 bytes --]

--- posix.c.~1.132.~	2004-07-14 10:19:36.000000000 +1000
+++ posix.c	2004-07-23 17:12:23.000000000 +1000
@@ -1426,20 +1426,46 @@
 #undef FUNC_NAME
 #endif /* HAVE_SYNC */
 
+
+/* crypt() returns a pointer to a static buffer, so we use scm_i_misc_mutex
+   to avoid another thread overwriting it.  A test program running crypt
+   continuously in two threads can be quickly seen tripping this problem.
+   crypt() is pretty slow normally, so a mutex shouldn't add much overhead.
+
+   glibc has a thread-safe crypt_r, but (in version 2.3.2) it runs a lot
+   slower (about 5x) than plain crypt if you pass an uninitialized data
+   block each time.  Presumably there's some one-time setups.  The best way
+   to use crypt_r to allow parallel execution in multiple threads would
+   probably be to maintain a little pool of initialized crypt_data
+   structures, take one and use it, then return it to the pool.  That pool
+   could be garbage collected so it didn't add permanently to memory use in
+   case only a few crypt calls are made.  But we expect crypt will be used
+   rarely, and even more rarely will there be any desire for lots of
+   parallel execution on multiple cpus.  So for now we don't bother with
+   anything fancy, just ensure it's thread-safe.  */
+
 #if HAVE_CRYPT
-SCM_DEFINE (scm_crypt, "crypt", 2, 0, 0, 
+SCM_DEFINE (scm_crypt, "crypt", 2, 0, 0,
             (SCM key, SCM salt),
 	    "Encrypt @var{key} using @var{salt} as the salt value to the\n"
 	    "crypt(3) library call.")
 #define FUNC_NAME s_scm_crypt
 {
-  char * p;
-
+  SCM ret;
   SCM_VALIDATE_STRING (1, key);
   SCM_VALIDATE_STRING (2, salt);
 
-  p = crypt (SCM_STRING_CHARS (key), SCM_STRING_CHARS (salt));
-  return scm_makfrom0str (p);
+  scm_frame_begin (0);
+  scm_frame_unwind_handler ((void(*)(void*)) scm_mutex_unlock,
+                            &scm_i_misc_mutex,
+                            SCM_F_WIND_EXPLICITLY);
+  scm_mutex_lock (&scm_i_misc_mutex);
+
+  ret = scm_makfrom0str (crypt (SCM_STRING_CHARS (key),
+                                SCM_STRING_CHARS (salt)));
+
+  scm_frame_end ();
+  return ret;
 }
 #undef FUNC_NAME
 #endif /* HAVE_CRYPT */

[-- Attachment #3: Type: text/plain, Size: 143 bytes --]

_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/guile-devel