On Thu, Mar 14, 2013 at 12:48:52PM -0400, Ted Unangst wrote:
> 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.
Oki; diff applied so I'm running with this now.
> 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;
> }
>
>
--
Antoine
