On Thu, Mar 14, 2013 at 17:24, Antoine Jacoutot wrote:
> On Thu, Mar 14, 2013 at 11:41:52AM -0400, Ted Unangst wrote:
>> On Thu, Mar 14, 2013 at 14:30, Antoine Jacoutot wrote:
>>
>> > FYI I am seeing a somehow similar crash when using sysutils/bacula (both
>> > 5.2 and 5.3).
>> > It is 100% reproducible on my setup. Obviously painful since it means I
>> > cannot run backups anymore...
>>
>> The following is brought to you without testing or warranty. It did
>> compile at least once though.
> 
> Awesome, thanks! I ran several batches of concurrent backups and I cannot
> reproduce the crash anymore :-)
> I'm going to run with that patch for the time being... if I spot any
> regression, I'll let you know.

Couple fixes. In some error cases, there are early returns I didn't
notice before. Fixed diff below, though I don't think a correct
program should be affected.

Alexey, sorry, I didn't get to your final diff before. It's very
similar to the diff below, so you were on the right track. One thing
that's different is you created unique special functions. That's only
necessary for malloc because it can't call pthread code (which in turn
calls malloc, creating a cycle). I did lock srandomdev. If a thread
stalls in sysctl, that's just bad luck. 

Index: stdlib/random.c
===================================================================
RCS file: /cvs/src/lib/libc/stdlib/random.c,v
retrieving revision 1.17
diff -u -p -r1.17 random.c
--- stdlib/random.c     1 Jun 2012 01:01:57 -0000       1.17
+++ stdlib/random.c     14 Mar 2013 16:40:15 -0000
@@ -36,6 +36,8 @@
 #include <stdlib.h>
 #include <unistd.h>
 
+#include "thread_private.h"
+
 /*
  * random.c:
  *
@@ -174,6 +176,12 @@ static int rand_type = TYPE_3;
 static int rand_deg = DEG_3;
 static int rand_sep = SEP_3;
 
+_THREAD_PRIVATE_MUTEX(random);
+static long random_l(void);
+
+#define LOCK() _THREAD_PRIVATE_MUTEX_LOCK(random)
+#define UNLOCK() _THREAD_PRIVATE_MUTEX_UNLOCK(random)
+
 /*
  * srandom:
  *
@@ -186,8 +194,8 @@ static int rand_sep = SEP_3;
  * introduced by the L.C.R.N.G.  Note that the initialization of randtbl[]
  * for default usage relies on values produced by this routine.
  */
-void
-srandom(unsigned int x)
+static void
+srandom_l(unsigned int x)
 {
        int i;
        int32_t test;
@@ -213,10 +221,18 @@ srandom(unsigned int x)
                fptr = &state[rand_sep];
                rptr = &state[0];
                for (i = 0; i < 10 * rand_deg; i++)
-                       (void)random();
+                       (void)random_l();
        }
 }
 
+void
+srandom(unsigned int x)
+{
+       LOCK();
+       srandom_l(x);
+       UNLOCK();
+}
+
 /*
  * srandomdev:
  *
@@ -234,6 +250,7 @@ srandomdev(void)
        int mib[2];
        size_t len;
 
+       LOCK();
        if (rand_type == TYPE_0)
                len = sizeof(state[0]);
        else
@@ -247,6 +264,7 @@ srandomdev(void)
                fptr = &state[rand_sep];
                rptr = &state[0];
        }
+       UNLOCK();
 }
 
 /*
@@ -273,12 +291,15 @@ initstate(u_int seed, char *arg_state, s
 {
        char *ostate = (char *)(&state[-1]);
 
+       LOCK();
        if (rand_type == TYPE_0)
                state[-1] = rand_type;
        else
                state[-1] = MAX_TYPES * (rptr - state) + rand_type;
-       if (n < BREAK_0)
+       if (n < BREAK_0) {
+               UNLOCK();
                return(NULL);
+       }
        if (n < BREAK_1) {
                rand_type = TYPE_0;
                rand_deg = DEG_0;
@@ -302,11 +323,12 @@ initstate(u_int seed, char *arg_state, s
        }
        state = &(((int32_t *)arg_state)[1]);   /* first location */
        end_ptr = &state[rand_deg];     /* must set end_ptr before srandom */
-       srandom(seed);
+       srandom_l(seed);
        if (rand_type == TYPE_0)
                state[-1] = rand_type;
        else
                state[-1] = MAX_TYPES*(rptr - state) + rand_type;
+       UNLOCK();
        return(ostate);
 }
 
@@ -333,6 +355,7 @@ setstate(char *arg_state)
        int32_t rear = new_state[0] / MAX_TYPES;
        char *ostate = (char *)(&state[-1]);
 
+       LOCK();
        if (rand_type == TYPE_0)
                state[-1] = rand_type;
        else
@@ -348,6 +371,7 @@ setstate(char *arg_state)
                rand_sep = seps[type];
                break;
        default:
+               UNLOCK();
                return(NULL);
        }
        state = &new_state[1];
@@ -356,6 +380,7 @@ setstate(char *arg_state)
                fptr = &state[(rear + rand_sep) % rand_deg];
        }
        end_ptr = &state[rand_deg];             /* set end_ptr too */
+       UNLOCK();
        return(ostate);
 }
 
@@ -376,8 +401,8 @@ setstate(char *arg_state)
  *
  * Returns a 31-bit random number.
  */
-long
-random(void)
+static long
+random_l(void)
 {
        int32_t i;
 
@@ -393,4 +418,14 @@ random(void)
                        rptr = state;
        }
        return((long)i);
+}
+
+long
+random(void)
+{
+       long r;
+       LOCK();
+       r = random_l();
+       UNLOCK();
+       return r;
 }


Reply via email to