How detect and avoid multi-threading conflict in a loop with OpenMP compilation?

Question

I am using an open-source simulation software that can be compiled with OpenMP-enabled cmake option. (https://github.com/oofem/oofem/)

In my class I am calling the following method in a for loop:

MaterialStatus *
Material :: giveStatus(GaussPoint *gp) const
/*
 * returns material status in gp corresponding to specific material class
 */
{
    MaterialStatus *status = static_cast< MaterialStatus * >( gp->giveMaterialStatus() );
    if ( status == nullptr ) {
        // create a new one
        status = this->CreateStatus(gp);

        // if newly created status is null
        // dont include it. specific instance
        // does not have status.
        if ( status ) {
            gp->setMaterialStatus( status );
        }
    }

    return status;
}

(here is the link of code above in Github repository.)

As you can see in the giveStatus method if there is no status already assigned, it will create and assign one, but if you call gp->setMaterialStatus(gp) and already there is a status instance assigned to gp the code will stop with an error.

Now my problem is if I don't compile the code with OpenMP the code will work fine, but if I use OpenMP-enabled compilation the code will stop with the error that the status is already assigned.

I am not sure what is happening, I think two objects try to get status from the same gp and since no status is assigned, the threads cannot obtain the *status pointer, and all of them try to set the status multiple times.

How can I obtain more information on this problem while debugging and how can I resolve this problem?

Answer 1

giveStatus is clearly not thread-safe. Thus, calling it from multiple threads in parallel cause a race-condition. Indeed, some threads can concurrently check if status is null and can enter the conditional in parallel. Then status is set by multiple thread causing an undefined behavior (typically an outcome that is dependent of the order of the execution of the threads). Because this code is not designed to be thread-safe, the simplest option is to put critical sections so to protect the code (ie. one thread will execute the section at a time and so the functions). This can be done using the directive #pragma omp critical. If you want the execution to be deterministic, it is probably better to use a #pragma omp master so to force the same unique thread to execute a code and then share its result to others using synchronizations (eg. barrier, atomic, critical sections, etc.).

Alternatively you can rewrite the code so to be thread-safe. To do so, you generally need not to use (hidden) state machines and favour thread-local storage. OOP code tends not to be great for that since a primary goal of OOP code is to abstract a state machine (through encapsulation).