j riv
j riv

Reputation: 3699

Why do compiler optimizations disrupt this functionality? (threads involved)

There's a thread that works normally with a loop

void* Thread (void* nothing) {

    while(1) {
        // Sleep if requested
        if ( Sleep_c)
                Sys_Sleep (Sleep_c);

        Do_stuff();
        if (shutdown) {
            shutdown_ok = 1;
            break;
        }
    }
    return 0;
}

The function on the main thread that may kill it does

 void Shutdown (void) {

    shutdown = 1;
    while (1) // Wait for it
        if (shutdown_ok) {
            shutdown = 0;
            break;
        }
 }

Now, this works fine on the debugger. But it gets stuck in the while(1) loop [of the shutdown function] in optimized code. Why?

Note: I should probably mutex lock the shared variables.

Upvotes: 0

Views: 202

Answers (5)

caf
caf

Reputation: 239041

The pthreads solution to this is to use a condition variable:

pthread_cond_t shutdown_cond = PTHREAD_COND_INITIALIZER;
pthread_mutex_t shutdown_lock = PTHREAD_MUTEX_INITIALIZER;

void* Thread (void* nothing) {

    while(1) {
        // Sleep if requested
        if ( Sleep_c)
                Sys_Sleep (Sleep_c);

        Do_stuff();

        pthread_mutex_lock(&shutdown_lock);
        if (shutdown) {
            shutdown_ok = 1;
            pthread_cond_signal(&shutdown_cond);
            pthread_mutex_unlock(&shutdown_lock);
            break;
        }
        pthread_mutex_unlock(&shutdown_lock);
    }
    return 0;
}

void Shutdown (void)
{
    pthread_mutex_lock(&shutdown_lock); 
    shutdown = 1;
    while (!shutdown_ok)
        pthread_cond_wait(&shutdown_cond, &shutdown_lock);
    shutdown = 0;
    pthread_mutex_unlock(&shutdown_lock);
}

In general, if you find yourself wanting to busy-loop, it's a sign that you should be using a condition variable.

Upvotes: 1

doron
doron

Reputation: 28892

The most likely reason for things not behaving the way that you expect is that the compiler is not expecting shutdown to change so it is happy to optimize it out. The solution is to use proper thread synchronization primitives like semaphores or condvars to get the behaviour that you are expecting.

Note

People will suggest making shutdown volatile and it is probably safe to assume writing to an int is atomic. This may help in probably most cases. But even so you may still get problems. On multicore machines reads and writes may be reordered unexpectedly leaving you to missing an important signal. Intel has a specialized LOCK instruction to deal with these cases but the compiler will not normally generate a LOCK. On ARM you have a DMB instruction but this as well is unlikely to be generated by the compiler.

Bottom line when doing thread synchronization, use the OS primitive and don't try to roll out your own. YOU WILL GET IT WRONG

Upvotes: 3

In silico
In silico

Reputation: 52159

Because the compiler does not realize that shutdown_ok will be modified outside the function in a different thread. Perhaps the compiler figured out that shutdown_ok will always evaluate to false inside the Shutdown() function and thus removed the if statement.

void Shutdown (void)
{        
    shutdown = 1;
    while (1)
    {
        shutdown = 0;
    }
}

One can mark a variable as volatile, which serves as a hint to the compiler that the variable can be modified in ways the compiler cannot predict.

C++ standard 7.1.5.1/8: [Note: volatile is a hint to the implementation to avoid aggressive optimization involve the object because the value of the object might be changed by means undetectable by an implementation... In general, the semantics of volatile are intended to be the same in C++ as they are in C.]

However, compilers may make volatile variables take on certain behaviors that are not specified in the standard. For example, Visual C++ compilers make volatile variables behave like memory locks, but such behavior is not actually guaranteed by any standard.

For this reason, volatile cannot be treated as a magic bullet that will solve all your multithreading problems. You're much better off using proper threading and concurrency primitives for this job.

Upvotes: 4

pagra
pagra

Reputation: 665

I think 'volatile' modifier for 'shutdown_ok' should avoid this. It tells the compiler that the variable can be modified by another thread so that it should always refer to it instead of using for example a register to hold a copy of its value.

Upvotes: 0

David Harris
David Harris

Reputation: 2340

What is the definition of shutdown? Where is it defined? I suspect that this needs to be a volatile variable.

Upvotes: 0

Related Questions