Describe the bug

Race conditions on getMonotonicUs between here and here. It may a false positive since it is reported by my static analysis tool. Thank you for your confirmation.

Here is the partial call stack leads to the method msUntilEarliestTimer executing here.

static void startBenchmarkThreads() {
    int i;
    for (i = 0; i < config.num_threads; i++) {
        benchmarkThread *t = config.threads[i];
        if (pthread_create(&(t->thread), NULL, execBenchmarkThread, t)){ // run aeMain
            fprintf(stderr, "FATAL: Failed to start thread %d.\n", i);
            exit(1);
        }
    }
   ...;
}

void aeMain(aeEventLoop *eventLoop) {
    eventLoop->stop = 0;
    while (!eventLoop->stop) {
        aeProcessEvents(eventLoop, AE_ALL_EVENTS|
                                   AE_CALL_BEFORE_SLEEP|
                                   AE_CALL_AFTER_SLEEP);      //run aeProcessEvents which calls msUntilEarliestTimer
    }
}

Here is the partial call stack leads to the method monotonicInit_posix executing here.

static void initBenchmarkThreads() {
    int i;
    if (config.threads) freeBenchmarkThreads();
    config.threads = zmalloc(config.num_threads * sizeof(benchmarkThread*));
    for (i = 0; i < config.num_threads; i++) {
        benchmarkThread *thread = createBenchmarkThread(i); //thread is created here and call aeCreateEventLoop
        config.threads[i] = thread;
    }
}
aeEventLoop *aeCreateEventLoop(int setsize) {
    aeEventLoop *eventLoop;
    int i;

    monotonicInit();    // call monotonicInit_posix which call monotonicInit_posix.

   ....;
}

To reproduce

Steps to reproduce the behavior and/or a minimal code sample.

Expected behavior

A description of what you expected to happen.

Additional information

Any additional information that is relevant to the problem.

Comment From: oranagra

@ycaibb thanks for reaching out. i see that one of the very first things that the main function in redis-benchmark.c does is call aeCreateEventLoop. so this will be the first call to monotonicInit (before there are any additional threads). this will set the global getMonotonicUs variable. and as far as i can tell, from that moment on, no one tries to set it if it's already set. i.e. only the monotonicInit_xxxx functions are setting it, and these are all called from monotonicInit and only in case it's still NULL.

please let me know if there's something that i'm missing.

Comment From: ycaibb

Hi, thank you for your help. Our tool reports below call traces.

main =>startBenchmarkThreads =>execBenchmarkThread=>aeMain=>aeProcessEvents=>msUntilEarliestTimer=>getMonotonicUs main=>benchmark=>initBenchmarkThreads=>createBenchmarkThread=>aeCreateEventLoop=>monotonicInit=>monotonicInit_posix

Comment From: oranagra

@ycaibb but do you agree that startBenchmarkThreads is only called after the initial init of monotonicInit that called from main is done, and in that case the global variable is set, and the call to monotonicInit_pisix in the stack trace you mentioned will never happen?

Comment From: ycaibb

OK, Thank you for your checking.