1

I have curious situation (at least for me :D ) in C++

My code is: 

static void startThread(Object* r){
    while(true)
    {
        while(!r->commands->empty())
        {
            doSomthing();
        }
    }
}

I start this function as thread using boost where commands in r is queue... this queue I fill up in another thread....

The problem is that if I fill the queue first and then start this tread everything works fine... But if I run the startThread first and after that I fill up queue commands, it is not working... doSomething() will not run...

Howewer if I modify startThread:

static void startThread(Object* r){
    while(true)
    {
        std::cout << "c" << std::endl;
        while(!r->commands->empty())
        {
            doSomthing();
        }
    }
}

I just added cout... and it is working... Can anybody explain why it is working with cout and not without? Or anybody has idea what can be wrong?

Maybe compiler is doing some kind of optimalization? I do not think so... :( Thanks

Dusan Plavak
  • 4,457
  • 4
  • 24
  • 35
  • If you send just empty queue, your thread ends soon. When you put a print, your parent thread gets a chance to fill the queue. Use some synchronization. Updating the queue from 2 threads may lead to undefined results if your queue is not thread safe. – Mohit Jain Apr 30 '14 at 10:36
  • It is "working" with cout because you have a bug that the slowdown it causes is masking. First of all, you need to protect access to the queues using some kind of synchronization mechanism. – molbdnilo Apr 30 '14 at 10:41
  • @Dusan Plavak - `But if I run the startThread first` And right there is the issue. You believed that when you call the function that starts the thread, the thread function is executed immediately. That is a false assumption. You cannot get away with writing a multithreaded program without using (or even learning) about proper synchronization, for example semaphores, mutexes, atomic operations, etc. – PaulMcKenzie Apr 30 '14 at 11:11
  • @PaulMcKenzie I did not assume which thread started first, I made sure that startThread started first... anyway the problem is as Konrad Rudolph described with link provided... – Dusan Plavak Apr 30 '14 at 11:20
  • @DusanPlavak - And how did you make sure that the startThread function was started first? It is not clear in your question as to how you started the function. Did you call the startThread function directly? `startThread(); FillUpTheQueue();` Did you do that? Or did you do this: `callThreadedFunction( startThread ); fillUpTheQueue()`? The `callThreadedFunction` is some generic function that starts up threads (doesn't matter if it's boost or not). – PaulMcKenzie Apr 30 '14 at 11:29

3 Answers3

3

But if I run the startThread first and after that I fill up queue commands, it is not working... doSomething() will not run

Of course not! What did you expect? Your queue is empty, so !r->commands->empty() will be false.

I just added cout... and it is working

You got lucky. cout is comparatively slow, so your main thread had a chance to fill the queue before the inner while test was executed for the first time.

So why does the thread not see an updated version of r->commands after it has been filled by the main thread? Because nothing in your code indicates that your variable is going to change from the outside, so the compiler assumes that it doesn’t.

In fact, the compiler sees that your r’s pointee cannot change, so it can just remove the redundant checks from the inner loop. When working with multithreaded code, you explicitly need to tell C++ that variables can be changed from a different context, using atomic memory access.

Community
  • 1
  • 1
Konrad Rudolph
  • 530,221
  • 131
  • 937
  • 1,214
  • I do not think that this is the case because Object r is pointer so it does not matter if commands was empty if I feel it later...? – Dusan Plavak Apr 30 '14 at 10:40
  • the outer `while` polls the `r->commands` list. but the compiler does not recognized, that the object is changed from "outside". you have to tell this the compiler by using volatile. – vlad_tepesch Apr 30 '14 at 10:47
  • @KonradRudolph Why is that? I am filling that queue from third tread and it is working OK, By OK I mean that in main thread the queue is filled up by third thread... – Dusan Plavak Apr 30 '14 at 10:47
  • the compiler cannot remove any loop, since this would change behaviour. if `empty` returns `false` there is an inner endless loop. if its `true`, there is an outer endless loop, because it caches the result of first execution since the compiler does not expect `r`to be changed from outside. – vlad_tepesch Apr 30 '14 at 10:52
  • @vlad_tepesch You’re right – but what it *can* do is remove the inner check by lifting it out of the loop. – Konrad Rudolph Apr 30 '14 at 10:55
  • @KonradRudolph thats true and thanks for the link. that if the compiler dos not support c++11 or in C? relying on on volatile and correct write backs of cpus caches seems the only option – vlad_tepesch Apr 30 '14 at 11:06
  • @KonradRudolph thanks for that link, so if I got it right all what I must to do is use some "memory barrier" - like mutex and it will work without volatile... :) thanks – Dusan Plavak Apr 30 '14 at 11:17
  • 1
    @Dusan Essentially yes. Just note that a mutex does two things, both necessary: it synchronises access, and it orders read and write operations (memory barrier), which, in turn, prevents the compiler from removing the read operation as an optimisation. – Konrad Rudolph Apr 30 '14 at 11:21
  • @vlad Before C++11, you are forced to rely on operating system or compiler specific primitives, pure C++ will simply not be enough. Some earlier compilers used to accept `volatile`, but modern compilers will potentially all break its usage for multi-threaded data access. – Konrad Rudolph Apr 30 '14 at 12:45
0

When u first run the thread and then fill up the queue, not entering the inner loop is logical, since the test !r->commands->empty() is true. After u add the cout statement, it is working because it takes some time to print the output, and meanwhile the other thread fills up the queue. so the condition becomes again true. But this is not good programming to rely on this facts in a multi-threading environment.

Rakib
  • 7,435
  • 7
  • 29
  • 45
  • But the outside while is wrote like: while(true) so it check inners loop condition again and again... – Dusan Plavak Apr 30 '14 at 10:42
  • @DusanPlavak then optimization by caching might be the reason.as I said this is not good programming to rely on this facts in a multi-threading environment. You should provide some synchronization primitives. – Rakib Apr 30 '14 at 10:49
0

There are two inter-related issues:

  • You are not forcing a reload of r->commands or r->commands-Yempty(), thus your compiler, diligent as it is in search of the pinnacle of performance, cached the result. Adding some more code might make the compiler remove this optimisation if it cannot prove the caching is still valid.
  • You have a data-race, so your program has undefined behavior. (I am assuming doSomething() removes an element and some other thread adds elements.

    1.10 Multi-threaded executions and data races § 21

    The execution of a program contains a data race if it contains two conflicting actions in different threads, at least one of which is not atomic, and neither happens before the other. Any such data race results in undefined behavior. [ Note: It can be shown that programs that correctly use mutexes and memory_order_- seq_cst operations to prevent all data races and use no other synchronization operations behave as if the operations executed by their constituent threads were simply interleaved, with each value computation of an object being taken from the last side effect on that object in that interleaving. This is normally referred to as “sequential consistency”. However, this applies only to data-race-free programs, and data-race-free programs cannot observe most program transformations that do not change single-threaded program semantics. In fact, most single-threaded program transformations continue to be allowed, since any program that behaves differently as a result must perform an undefined operation. —end note ] 22

Deduplicator
  • 44,692
  • 7
  • 66
  • 118