Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix zero-length query call for DispatchQueue::Barrier #507

Merged
merged 1 commit into from
May 18, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
21 changes: 14 additions & 7 deletions src/autowiring/test/DispatchQueueTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ TEST_F(DispatchQueueTest, SimpleEvents) {

int num = DispatchAllEvents();

*this += [&count] () {
*this += [&count]() {
count += 5 ;
};

Expand Down 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 @@ -109,25 +116,25 @@ TEST_F(DispatchQueueTest, BarrierWithAbort) {
std::lock_guard<std::mutex> lk(b->lock);
};

// Delay for long enough for the barrier to be reached:
std::this_thread::sleep_for(std::chrono::milliseconds(1));

// 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;
}
);

// Delay for long enough for the barrier to be reached:
std::this_thread::sleep_for(std::chrono::milliseconds(1));

// 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_FALSE(f.get()) << "Exception should have been thrown inside the Barrier call";
}