Why does thread not recognize change of a flag?

Question

I have a strange situation under C/Visual Studio on a Windows 7 platform. There is a problem from time to time and I spent a lot of time to find it. The problem is within a third party library, for which I have the complete code. There a thread is created (the printLog statements are from myself):

    ...
    plafParams->eventThreadFlag = 2;
    printLog("before CreateThread");
    if (plafParams->hReadThread_p = CreateThread(NULL, 0, ( LPTHREAD_START_ROUTINE ) plafPortReadThread, ( void * ) dlmsInstance, 0,
                                                 &plafParams->portReadThreadID) )
    {
        printLog("after CreateThread: OK");
        plafParams->eventThreadFlag = 3;
    }
    else
    {
        unsigned int lasterr = GetLastError();
        printLog("error CreateThread, last error:%x", lasterr);
        /* Could not create the read thread. */
        ...
        ...
        return FAILURE;
    }

    printLog("SUCCESS");
    ...
    ...

The thread function is:

    void *plafPortReadThread(DLMS_GLOBALS *dlmsInstance)
    {

        PLAF_PARAMS *plafParams;


        plafParams = (PLAF_PARAMS *)(dlmsInstance->plafParams);
        printLog("start - plafPortReadThread, plafParams->eventThreadFlag=%x", plafParams->eventThreadFlag);

        while ((plafParams->eventThreadFlag != 1) && (plafParams->eventThreadFlag != 3))
        {
            if (plafParams->eventThreadFlag == 0)
            {
                printLog("exit 1 - plafPortReadThread, plafParams->eventThreadFlag=%x", plafParams->eventThreadFlag);
                CloseHandle(plafParams->hReadThread_p);
                plafFree((void **)&plafParams);
                ExitThread(0);
               break;
            }
        }
        printLog("start - plafPortReadThread, proceed=%d", proceed);
        ...

Now, when the flag is set before the while loop is started within the thread, everything works OK:

SUCCESS
start - plafPortReadThread, plafParams->eventThreadFlag=3

But sometimes the thread is quick enough so the while loop is started before the flag is actually set within the outer part.

The output is then:

start - plafPortReadThread, plafParams->eventThreadFlag=2
SUCCESS

Most surprisingly the while loop doesn't exit, even after the flag has been set to 3.

It seems, that the compiler "optimizes" the flag and assumes, that it cannot be changed from outside.

What could be the problem? I'm really surprised. Or is there something else I have overseen completely? I know, that the code is not very elegant and that such things should better be done with semaphores or signals. But it is not my code and I want to change as little as possible.

After removing the whole while condition it works as expected. Should I change the struct or its fields to volatile ? Everybody says, that volatile is useless in our days and not needed anymore, except in the case, where a memory location is changed by peripherals...

The behaviour observe is indeed an optimization. `volatile` is not useless and this is may be what you need. But probably it won't be enough and you'll need locks/atomic flags to ensure correct access to shared data. — freakish, Jul 09 '18 at 09:04
I don't know - the compiler surely shouldn't be optimising that away completely? Volatile ought to sort it out if that truly is the cause, but we shouldn't be needing to second guess to that degree how much of our code is ignored by the compiler .... Also, why is not logging the post thread creation log messages? — noelicus, Jul 09 '18 at 09:12
Technically, I believe it needs `_Atomic` not `volatile`. Without `_Atomic` it should be UB, but I still think it should work on common platforms (x86). — Petr Skocik, Jul 09 '18 at 09:16
`volatile` won't help you in the general case (when dealing with threads running on different cores eg.). What you need, is a [memory barrier](https://en.wikipedia.org/wiki/Memory_barrier). Typically by using a mutex or semaphore. — Sander De Dycker, Jul 09 '18 at 09:16
Most interestingly not even in the answers there is a homogeneous opinion about "volatile". Is it useless now or not? — MichaelW, Jul 09 '18 at 09:29
@michael : `volatile` is not intended for multi-threaded code, so in most cases, it [won't be useful for multi-threaded synchronization](https://stackoverflow.com/questions/2484980/why-is-volatile-not-considered-useful-in-multithreaded-c-or-c-programming). It is however still [useful for its intended purpose](https://stackoverflow.com/questions/246127/why-is-volatile-needed-in-c). — Sander De Dycker, Jul 09 '18 at 09:37
@michael - Historically, Visual Studio has had *an extension* making volatile useful for multi-threading. Kind of a hack from when atomics were not available. However, this use is on its way out [with an option to turn it on or off](https://learn.microsoft.com/en-us/cpp/build/reference/volatile-volatile-keyword-interpretation). So definitely not recommended for new code. — Bo Persson, Jul 09 '18 at 10:20
Too much UB in this snippet. Unsynchronized access to a shared variable is UB, the (LPTHREAD_START_ROUTINE) cast is UB. Relying on UB includes it working by accident. Declaring the variable *volatile* is one such accident, tends to work on a processor with a strong memory model, like x86. The cast works by accident on x64. Work with the programmer of this code to get these bugs resolved. — Hans Passant, Jul 09 '18 at 12:32

midor · Accepted Answer · 2018-07-09T11:36:45.703

Prior to C11 this is totally platform-dependent, because the effect you are observing is due to the memory model used by your platform. This is different from a compiler optimization as synchronization points between threads require the compiler to insert barrier instructions, instead of, e.g., making something a constant. For C11 for section 7.17.3 specifies the different models. So your value is not optimized out statically, thread A just never reads the value thread B wrote, but still has its local value.

In practice many projects don't use C11 yet, and thus you will likely have to check the documentation of your platform. Note that in many cases you don't have to modify the type of the variable for the flag (in case you can't). Most memory models specify synchronization points that also forbid reordering of certain instructions, i.e. in:

int x = 3;
_Atomic int a = 1;
x = 5;
a = 2;

the compiler will often have to ensure that x has the value 3 when a has the value 1, and that when a is assigned the value 2, x will have the value 5. volatile does not participate in this relationship (in the C/C++ 11 models - often confused because it does participate in Java's happened-before), and is mostly useless, unless your writes should never be optimized out because they have side-effects such as a LED blinking which the compiler can't understand:

volatile int x = 1; // some special location - blink then clear 
x = 1; // blink then clear
x = 1; // blink then clear

Why does thread not recognize change of a flag?

1 Answers1