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.

- _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();

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.

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