Skip to content

Commit

Permalink
Prevent hiding lock from base class
Browse files Browse the repository at this point in the history
  • Loading branch information
Jonathan Marsden committed Nov 7, 2017
1 parent 3c2a4bd commit c12221e
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 14 deletions.
20 changes: 10 additions & 10 deletions src/autowiring/AutoPacketFactory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,15 @@ bool AutoPacketFactory::IsRunning(void) const {
}

std::shared_ptr<AutoPacket> AutoPacketFactory::CurrentPacket(void) {
std::lock_guard<std::mutex> lk(m_lock);
std::lock_guard<std::mutex> lk(m_apfLock);
return m_curPacket.lock();
}

std::shared_ptr<AutoPacket> AutoPacketFactory::NewPacket(void) {
std::shared_ptr<AutoPacketInternal> retVal;
bool isFirstPacket;
{
std::lock_guard<std::mutex> lk(m_lock);
std::lock_guard<std::mutex> lk(m_apfLock);

if (ShouldStop())
throw autowiring_error("Attempted to create a packet on an AutoPacketFactory that was already terminated");
Expand Down Expand Up @@ -84,14 +84,14 @@ std::shared_ptr<void> AutoPacketFactory::GetInternalOutstanding(void) {
}

std::vector<AutoFilterDescriptor> AutoPacketFactory::GetAutoFilters(void) const {
std::lock_guard<std::mutex> lk(m_lock);
std::lock_guard<std::mutex> lk(m_apfLock);
std::vector<AutoFilterDescriptor> retVal;
retVal.assign(m_autoFilters.begin(), m_autoFilters.end());
return retVal;
}

SatCounter* AutoPacketFactory::CreateSatCounterList(void) const {
std::lock_guard<std::mutex> lk(m_lock);
std::lock_guard<std::mutex> lk(m_apfLock);

// Trivial return check
if (m_autoFilters.empty())
Expand All @@ -113,7 +113,7 @@ SatCounter* AutoPacketFactory::CreateSatCounterList(void) const {

bool AutoPacketFactory::OnStart(void) {
// Initialize first packet
std::lock_guard<std::mutex>{m_lock},
std::lock_guard<std::mutex>{m_apfLock},
m_nextPacket = ConstructPacket();
return true;
}
Expand All @@ -124,7 +124,7 @@ void AutoPacketFactory::OnStop(bool graceful) {
std::shared_ptr<AutoPacketInternal> nextPacket;

// Lock destruction precedes local variables
std::lock_guard<std::mutex>{m_lock},
std::lock_guard<std::mutex>{m_apfLock},
autoFilters.swap(m_autoFilters),
nextPacket.swap(m_nextPacket);
}
Expand Down Expand Up @@ -156,14 +156,14 @@ void AutoPacketFactory::Clear(void) {
}

const AutoFilterDescriptor& AutoPacketFactory::AddSubscriber(const AutoFilterDescriptor& rhs) {
std::lock_guard<std::mutex> lk(m_lock);
std::lock_guard<std::mutex> lk(m_apfLock);
m_autoFilters.insert(rhs);
return rhs;
}

void AutoPacketFactory::RemoveSubscriber(const AutoFilterDescriptor& autoFilter) {
// Trivial removal from the autofilter set:
std::lock_guard<std::mutex> lk(m_lock);
std::lock_guard<std::mutex> lk(m_apfLock);
m_autoFilters.erase(autoFilter);
}

Expand All @@ -186,7 +186,7 @@ size_t AutoPacketFactory::GetOutstandingPacketCount(void) const {
}

void AutoPacketFactory::RecordPacketDuration(std::chrono::nanoseconds duration) {
std::unique_lock<std::mutex> lk(m_lock);
std::unique_lock<std::mutex> lk(m_apfLock);
m_packetDurationSum += duration.count();
m_packetDurationSqSum += duration.count() * duration.count();
}
Expand All @@ -202,7 +202,7 @@ double AutoPacketFactory::GetPacketLifetimeStandardDeviation(void) {
}

void AutoPacketFactory::ResetPacketStatistics(void) {
std::unique_lock<std::mutex> lk(m_lock);
std::unique_lock<std::mutex> lk(m_apfLock);
m_packetCount = 0;
m_packetDurationSum = 0.0;
m_packetDurationSqSum = 0.0;
Expand Down
4 changes: 2 additions & 2 deletions src/autowiring/AutoPacketFactory.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ class AutoPacketFactory:

private:
// Lock for this type
mutable std::mutex m_lock;
mutable std::mutex m_apfLock;

// Internal outstanding reference for issued packet:
std::weak_ptr<void> m_outstandingInternal;
Expand Down Expand Up @@ -60,7 +60,7 @@ class AutoPacketFactory:
/// </summary>
template<class T>
void AppendAutoFiltersTo(T& container) const {
std::lock_guard<std::mutex> lk(m_lock);
std::lock_guard<std::mutex> lk(m_apfLock);
container.insert(container.end(), m_autoFilters.begin(), m_autoFilters.end());
}

Expand Down
9 changes: 7 additions & 2 deletions src/autowiring/CoreRunnable.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,11 @@ bool CoreRunnable::Start(std::shared_ptr<CoreObject> outstanding) {
return true;

m_wasStarted = true;
m_outstanding = outstanding;
m_outstanding = std::move(outstanding);
outstanding.reset();
if(!OnStart()) {
m_shouldStop = true;

m_outstanding.reset();

// Immediately invoke a graceless stop in response
Expand All @@ -38,20 +40,23 @@ bool CoreRunnable::Start(std::shared_ptr<CoreObject> outstanding) {
}

void CoreRunnable::Stop(bool graceful) {
std::unique_lock<std::mutex> lk(m_lock);
if (!m_shouldStop) {
// Stop flag should be pulled high
m_shouldStop = true;

// Do not call this method more than once:
lk.unlock();
OnStop(graceful);
lk.lock();
}

if (m_outstanding) {
// Ensure we do not invoke the outstanding count dtor while holding a lock
std::shared_ptr<CoreObject> outstanding;
std::lock_guard<std::mutex>{m_lock},
outstanding.swap(m_outstanding);
}
lk.unlock();

// Everything looks good now
m_cv.notify_all();
Expand Down

0 comments on commit c12221e

Please sign in to comment.