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 have not used any pragma code to use multi-threading in my loop. I am not sure what is happening, I think the threads cannot obtain the *status pointer, and all of them try to set the status multiple times, another possibility is that the pointer *status somehow conflicts, but I don't know how to avoid this problem.

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

CodePudding user response：

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).

CodePudding user response：

The problem in your code is that a Material object acts on a GaussPoint and without further context there is no way to see if two materials don't act on the same Gauss point. Hence you have a race condition. For a parallelizable code you need to turn this code sideways and consider all Gauss points and ask them for their interaction with the materials.

In my own way of phrasing this, you are using a "push" model, where one object pushes state onto several objects of some other class. For a thread-safe formulation you need to transpose this and use a "pull" model where the other class objects gather all the actions on themselves. Why do I say "transpose"? Well, think of it as a very abstract description of a matrix-vector multiplication. In the "pull" model each output point is an inner product (of a matrix row) with the whole source vector. In the "push" model each input point adds (a scaling of a matrix column) to the whole output. So the one is the transpose product formulation of the other.

And there's a bit of parallelism philosophy for you.

Oh, and as to your direct question: OpenMP lets you shoot yourself in the foot. It will not detect conflicts. You have to avoid them yourself.