Possible Race Condition due to Missing Barrier After Incrementing numTS

[biergaizi]

I suspect the current engine_sse_compressed.cpp is unsafe due to a missing synchronization barrier, although I can't actually reproduce the problem.

for (unsigned int iter=0; iter<m_enginePtr->m_iterTS; ++iter)
{
        // pre voltage stuff...
        m_enginePtr->DoPreVoltageUpdates(m_threadID);

        //voltage updates
        m_enginePtr->UpdateVoltages(m_start,m_stop-m_start+1);

        // [...]
        // function body omitted
        // [...]

        if (m_threadID == 0)
                ++m_enginePtr->numTS; // only the first thread increments numTS
}

There is no barrier after ++m_enginePtr->numTS. Thus, if other threads finish before the first thread, the next iteration would be immediately started. At this point, the first thread may finish, and it's possible that numTS will be changed in the middle of DoPreVoltageUpdates().

As far as I see, only the Excitation extension relies on the value of numTS, so this race condition, if occurs at rare occasion, would only affect the very early stage of the simulation. So it would be difficult to see the problem in practice.

[biergaizi]

After reviewing the code, I now see why the issue does not occur in practice. This is the result of the priority system.

// priority definitions for some important extensions
#define ENG_EXT_PRIO_STEADYSTATE                +2e6  //steady state extension priority
#define ENG_EXT_PRIO_UPML                               +1e6  //unaxial pml extension priority
#define ENG_EXT_PRIO_CYLINDER                   +1e5  //cylindrial extension priority
#define ENG_EXT_PRIO_TFSF                               +5e4  //total-field/scattered-field extension priority
#define ENG_EXT_PRIO_EXCITATION                 -1000 //excitation priority
#define ENG_EXT_PRIO_CYLINDERMULTIGRID  -3000 //cylindrial multi-grid extension priority

Because only the Excitation extension uses the global timestep value, and because the Excitation extension does not actually use the DoPreVoltageUpdates() method, it's getting synchronized anyway...

In conclusion, this would only be a problem when:

1. The extension has the highest priority.
2. The extension uses DoPreVoltageUpdates()

So it never occurs in any practical use. But it can be a potential problem if one develops a new extension.