Re: [Boost-users] Threading question

Subject: Re: [Boost-users] Threading question
From: Ulf Samuelsson (boost-user_at_[hidden])
Date: 2013-12-13 18:24:17

2013-12-13 18:51, Gottlob Frege skrev:
>
>
>
> On Fri, Dec 13, 2013 at 9:25 AM, Ulf Samuelsson <boost-user_at_[hidden] 
> <mailto:boost-user_at_[hidden]>> wrote:
>
>     Typical code.
>
>     bool Server::QueueEmpty(void) {
>         bool empty;
>         try {
>             THREAD_WRITE("Checking SendQueue", YELLOW);
>             empty = true;
>             while (1) {
>                 boost::mutex::scoped_lock lock(Queue_Lock,
>     boost::try_to_lock);
>                 if (lock) {
>                     empty = SendQueue.empty();
>                     break;
>                 } else {
>                     // Didn't get lock, retry after yield
>                     THREAD_WRITE("Sendstart Yielding QueueEmpty", RED);
>                     this_thread::yield();
>                     THREAD_WRITE("Sendstart Returning QueueEmpty", RED);
>                 }
>             }
>         } catch (std::exception &e) {
>             DebugPort.WriteLine(e.what(), RED);
>             empty = true;
>         }
>         return empty;
>     }
>
>     Is there an obvious problem with this code?
>
>
> - Asking if a multithreaded queue is empty is typically useless. 
>  Whatever value it returns is already out of date.

No, because this is a producer / consumer scenario.
The producer will only add to the queue.
The consumer is the only thread removing stuff from the queue,
     and is also the only thread to call QueueEmpty.

If the Queue tests to be non-empty, then it will also be non-empty when
    the the returned value will be used.

If the Queue tests to be empty, and a value is inserted between the test,
and the use of the returned value, it is not a problem. since the recently
added values will be sent the next time through the loop.

The main loop
loop
     empty = QueueEmpty();
     ...
     if (not empty and other conditions)
          prepare data
          send data to webserver
          remove stuff form the Queue
    end if;
    Debug Output
end loop;

>
> - _trying_ to grab a lock, then yielding, then retrying, can be 
> written much shorter:
>
> // get the lock, (which might require waiting/yielding until you can 
> get it)
> boost::mutex::scoped_lock lock(Queue_lock);
> return SendQueue.empty();
>
Yes, but I would like to understand more about the run time behaviour,
and the code will allow the Debug Output to be enabled.
>
>
>     It is using Yield, which I assumed was the right way to release
>     the CPU.
>     Using sleep feels wrong, since I do not want to sleep, I want to
>     wait for the mutex.
>
>
> If you want to wait for the mutex, wait for the mutex.  Pass it to 
> scoped_lock and it will wait for you.
>
>     The first thread implements the mutex in the same way, although
>     not in a subroutine.
>
>
> If all your code uses mutexes similarly, well, I think you are 
> fundamentally using mutexes wrong, so the bug could be anywhere.
> I mean, your example code above isn't technically wrong (that I can 
> see), but it just isn't how mutexes are meant to be used.  What you 
> are doing is overly complicated, and complicated tends to lead to 
> bugs.  Keep it simple.
>
I agree in principle, but as I am trying to figure out what goes wrong, 
I need debug printouts
and this is what I ended up with.

Before I had one biocking mutex test in the consumer, and one in the 
producer,
in the beginning of the loop.
Then the application hung within hours.
Now it took a week for it to hang.

>     Am I correct in assuming, that since I use scoped_lock, the mutex will
>     always be released when I exit the while loop with "break;"?
>
>
> yes.  Even with an exception thrown, it will always release the lock.
>
>
> Tony
>
>
>
> _______________________________________________
> Boost-users mailing list
> Boost-users_at_[hidden]
> http://lists.boost.org/mailman/listinfo.cgi/boost-users