0

The below code (and which is compilable as is) results in the random number generator returning the very same random number for all processes for some reason. How could that be? Am I doing something wrong with the mutex?

#include <sys/types.h>
#include <sys/wait.h>
#include <pthread.h>
#include <unistd.h>
#include <stdlib.h>
#include <stdio.h>
#include <time.h>

#define RETURN_FAILURE_IF_TRUE(condition, ...) \
{ \
    if(condition) \
    { \
        fprintf(stderr, __VA_ARGS__); \
        return EXIT_FAILURE; \
    } \
}

#define RETURN_FAILURE_IF_FALSE(condition, ...) \
    RETURN_FAILURE_IF_TRUE(!(condition), __VA_ARGS__)

pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER;

int nextRandomDouble(double* d)
{
    if(pthread_mutex_lock(&mutex) != 0) return 0;
    *d = drand48();
    if(pthread_mutex_unlock(&mutex) != 0) return 0;
    return 1;
}

int main()
{
    const int processes = 5;
    srand48(time(NULL));

    for(int i = 0; i < processes; ++i)
    {
        pid_t pid = fork();
        RETURN_FAILURE_IF_TRUE(pid < 0, "Fork failed.\n");
        if(pid == 0)
        {
            double d;
            RETURN_FAILURE_IF_FALSE(nextRandomDouble(&d), "PRNG failed.\n");
            printf("rnd: %f\n", d);
            return EXIT_SUCCESS;
        }
    }

    for(int i = 0; i < processes; ++i)
    {
        int status;
        pid_t pid = waitpid(-1, &status, 0);
        RETURN_FAILURE_IF_TRUE(
            (pid != 1) && (status != 0), "Child exit failed.\n"
        );
    }
    return EXIT_SUCCESS;
}
aandeers
  • 431
  • 1
  • 6
  • 19

3 Answers3

4
srand48(time(NULL));

You seed the PRNG in each process with the time the process started, to the second. This means that all processes that start in the same second seed the PRNG with the same value.

Try:

srand48((getpid()*2654435761U)^time(NULL));
David Schwartz
  • 179,497
  • 17
  • 214
  • 278
  • Can you enlighten us on what this magic number is, and is for ? – Alexandre C. Nov 08 '11 at 20:08
  • 1
    It comes from [Knuth](http://stackoverflow.com/questions/664014/what-integer-hash-function-are-good-that-accepts-an-integer-hash-key/665545#665545). It's a simple way to mix the bits of the PID so that PIDs that differ only by a few bits on the end won't cancel out against times that only differ by a few seconds. 9802->F819732A 9803->9650ECDB 9804->3488668C 9805->D2BFE03D. Notice how many bits differ? – David Schwartz Nov 08 '11 at 20:11
  • Clever. A quick google for the constant turned out to be productive in the meantime. For reference, this is enlightening: http://mathforum.org/kb/thread.jspa?messageID=431065&tstart=0 – Alexandre C. Nov 08 '11 at 20:18
  • Just using `clock_gettime` and passing nanoseconds rather than seconds as the seed would work a lot better, I think... – R.. GitHub STOP HELPING ICE Nov 08 '11 at 20:24
  • @R..: Assuming the clock has adequate resolution. It should have TSC resolution on a typical, modern PC, which is certainly adequate. – David Schwartz Nov 08 '11 at 20:25
  • Nice magic, but kind of useless if you use `srand`. This has an internal state that is usually shared between threads. – Jens Gustedt Nov 08 '11 at 22:19
  • @JensGustedt: Huh? You still have the issue of making sure different processes get different strings of random numbers. – David Schwartz Nov 08 '11 at 23:21
  • @DavidSchwartz, sure but what concerns initialization with `srand48` it is just the last thread that run the initialization that wins. So calling it for all threads makes no sense at all. You've got to use a PRG that uses different states per thread, please see my answer. – Jens Gustedt Nov 09 '11 at 06:50
  • @JensGustedt: In any sane multi-threaded use of `srand48`, only one thread will initialize the PRNG. I see no special advantage to different states per thread versus one state for all threads, assuming proper locking. – David Schwartz Nov 09 '11 at 09:24
  • @DavidSchwartz, locking has a cost and makes an application that uses a lot of random numbers basically sequential. Then there is a more subtle thing, the numbers you get can not be considered independent between the different threads, for things like Monte Carlo simulations this can be a killer. – Jens Gustedt Nov 09 '11 at 09:56
  • @JensGustedt: Of course this kind of simple PRNG is unsuitable for some applications. That really has nothing to do with the threading issue. The same argument would apply if the simulation used a single thread. And I disagree about the cost of locking. Locking deschedules contending threads and is basically free when non-contending threads are running. That you can do something badly is an argument not to do it badly, not an argument not to do it. – David Schwartz Nov 09 '11 at 10:32
1

You get the same sequence of random numbers in each process because you seed the PRNG before the call to fork(). After the call to fork(), each process has it's own copy of the PRNG, seeded to the same value - so of course each process gets the same sequence of numbers.

Note that the mutex calls are unnecessary, because after a fork() each process is operating in its own virtual address space - there's no shared state between the processes here.

If you use pthread_create() instead of fork(), creating separate threads, then the threads will share the PRNG state and they will each get a different value from the PRNG sequence:

void *thread_func(void *arg)
{
    double d;
    if (!nextRandomDouble(&d))
        fprintf(stderr, "PRNG failed.\n");
    else
        printf("rnd: %f\n", d);
    return 0;
}

int main()
{
    const int processes = 5;
    pthread_t thread[processes];
    srand48(time(NULL));

    for(int i = 0; i < processes; ++i)
    {
        int pthread_err = pthread_create(&thread[i], NULL, thread_func, NULL);
        RETURN_FAILURE_IF_TRUE(pthread_err != 0, "pthread_create failed.\n");
    }

    for(int i = 0; i < processes; ++i)
    {
        void *status;
        int pthread_err = pthread_join(thread[i], &status);
        if ((pthread_err != 0) || (status != 0))
            fprintf(stderr, "Child exit failed.\n");
    }
    return EXIT_SUCCESS;
}
caf
  • 233,326
  • 40
  • 323
  • 462
0

Using drand48 is a bad idea in a multi-threaded environment. This is using a shared globale state between the calls. So either the threads step on the feet of each other (if this isn't made thread safe) or wait endlessly for their turn (if the access is mutexed, as you did).

Use erand48 instead. This receives the state in the argument that you should initialize to a different value for each thread.

Jens Gustedt
  • 76,821
  • 6
  • 102
  • 177