2013-12-13 18:51, Gottlob Frege skrev:

On Fri, Dec 13, 2013 at 9:25 AM, Ulf Samuelsson <boost-user@emagii.com> 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@lists.boost.org
http://lists.boost.org/mailman/listinfo.cgi/boost-users