Skip to content

Commit

Permalink
Fix zero-length query call for DispatchQueue::Barrier
Browse files Browse the repository at this point in the history
When Barrier is called with a value of zero, the return value must be set according to the number of elements in the queue as of the time of the call.  Not only is this a good optimization, but it's also a way to allow consumers to easily poll the status of the queue.

Fixes #504
  • Loading branch information
codemercenary committed May 16, 2015
1 parent 64e4d0b commit 82f4c9b
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 8 deletions.
3 changes: 3 additions & 0 deletions autowiring/DispatchQueue.h
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,9 @@ class DispatchQueue {
///
/// If DispatchQueue::Abort() is called before the dispatcher has been completed, this method will throw an exception.
/// If a dispatcher on the underlying DispatchQueue throws an exception, this method will also throw an exception.
///
/// If zero is passed as the timeout value, this method will return true if and only if the queue was empty at the time
/// of the call, ignoring any delayed dispatchers.
/// </remarks>
bool Barrier(std::chrono::nanoseconds timeout);

Expand Down
28 changes: 23 additions & 5 deletions src/autowiring/DispatchQueue.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -177,15 +177,33 @@ void DispatchQueue::AddExisting(DispatchThunkBase* pBase) {
}

bool DispatchQueue::Barrier(std::chrono::nanoseconds timeout) {
// Set up the lambda:
auto complete = std::make_shared<bool>(false);
*this += [complete] { *complete = true; };
static const char text [] = "Dispatch queue was aborted while a barrier was invoked";

// Obtain the lock, wait until our variable is satisfied, which might be right away:
// Optimistic check first:
std::unique_lock<std::mutex> lk(m_dispatchLock);

// Short-circuit if dispatching has been aborted
if (m_aborted)
throw dispatch_aborted_exception("Dispatch queue was aborted before a timed wait was attempted");

// Short-circuit if the queue is already empty
if (m_dispatchQueue.empty())
return true;

// Also short-circuit if zero is specified as the timeout value
if (timeout.count() == 0)
return false;

// Set up the lambda. Note that the queue size CANNOT be 1, because we just checked to verify
// that it is non-empty. Thus, we do not need to signal the m_queueUpdated condition variable.
auto complete = std::make_shared<bool>(false);
auto lambda = [complete] { *complete = true; };
m_dispatchQueue.push_back(new DispatchThunk<decltype(lambda)>(std::move(lambda)));

// Wait until our variable is satisfied, which might be right away:
bool rv = m_queueUpdated.wait_for(lk, timeout, [&] { return m_aborted || *complete; });
if (m_aborted)
throw dispatch_aborted_exception("Dispatch queue was aborted while a barrier was invoked");
throw dispatch_aborted_exception("Dispatch queue was aborted during a timed wait");
return rv;
}

Expand Down
13 changes: 10 additions & 3 deletions src/autowiring/test/DispatchQueueTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,13 @@ TEST_F(DispatchQueueTest, PathologicalStartAndStop){
ASSERT_TRUE(t4->WaitFor(std::chrono::seconds(10)));
}

TEST_F(DispatchQueueTest, TrivialBarrier) {
AutoCurrentContext()->Initiate();
AutoRequired<CoreThread> ct;

ASSERT_TRUE(ct->Barrier(std::chrono::seconds(0))) << "Zero-time barrier on a zero-length queue did not pass as expected";
}

TEST_F(DispatchQueueTest, Barrier) {
AutoCurrentContext()->Initiate();
AutoRequired<CoreThread> ct;
Expand Down Expand Up @@ -110,16 +117,16 @@ TEST_F(DispatchQueueTest, BarrierWithAbort) {
};

// Launch something that will barrier:
auto exception = std::make_shared<bool>(false);
auto f = std::async(
std::launch::async,
[=] {
try {
ct->Barrier(std::chrono::seconds(5));
}
catch (autowiring_error&) {
*exception = true;
return false;
}
return true;
}
);

Expand All @@ -129,5 +136,5 @@ TEST_F(DispatchQueueTest, BarrierWithAbort) {
// Now abandon the queue, this should cause the async thread to quit:
ct->Abort();
ASSERT_EQ(std::future_status::ready, f.wait_for(std::chrono::seconds(5))) << "Barrier did not abort fast enough";
ASSERT_TRUE(*exception) << "Exception should have been thrown inside the Barrier call";
ASSERT_TRUE(f.get()) << "Exception should have been thrown inside the Barrier call";
}

0 comments on commit 82f4c9b

Please sign in to comment.