From f9733888799ab2d6fa03f414ccae87b5d68406cf Mon Sep 17 00:00:00 2001 From: Andrey Prygunkov Date: Wed, 16 Mar 2016 22:37:19 +0100 Subject: [PATCH] #185: use guards with private mutexes Use guard objects to automatically unlock private mutexes when leaving current scope. --- daemon/connect/Connection.cpp | 9 +-- daemon/connect/WebDownloader.cpp | 6 +- daemon/extension/QueueScript.cpp | 17 ++--- daemon/feed/FeedCoordinator.cpp | 102 +++++++++++++----------------- daemon/main/Maintenance.cpp | 11 ++-- daemon/main/Scheduler.cpp | 17 +++-- daemon/nntp/ArticleDownloader.cpp | 6 +- daemon/nntp/ArticleWriter.cpp | 25 ++++---- daemon/nntp/ServerPool.cpp | 24 ++----- daemon/nntp/StatMeter.cpp | 29 ++------- daemon/postprocess/ParChecker.cpp | 64 ++++++++++--------- daemon/queue/DownloadInfo.cpp | 7 +- daemon/queue/Scanner.cpp | 18 +++--- daemon/util/Log.cpp | 32 +++------- daemon/util/Script.cpp | 9 +-- daemon/util/Thread.cpp | 26 ++++---- daemon/util/Thread.h | 13 ++++ 17 files changed, 180 insertions(+), 235 deletions(-) diff --git a/daemon/connect/Connection.cpp b/daemon/connect/Connection.cpp index f22c4489..86f7aa91 100644 --- a/daemon/connect/Connection.cpp +++ b/daemon/connect/Connection.cpp @@ -970,24 +970,17 @@ in_addr_t Connection::ResolveHostAddr(const char* host) err = hinfo == nullptr; #endif #else - m_getHostByNameMutex->Lock(); + Guard guard(m_getHostByNameMutex); hinfo = gethostbyname(host); err = hinfo == nullptr; #endif if (err) { -#ifndef HAVE_GETHOSTBYNAME_R - m_getHostByNameMutex->Unlock(); -#endif ReportError("Could not resolve hostname %s", host, true, h_errnop); return INADDR_NONE; } memcpy(&uaddr, hinfo->h_addr_list[0], sizeof(uaddr)); - -#ifndef HAVE_GETHOSTBYNAME_R - m_getHostByNameMutex->Unlock(); -#endif } return uaddr; } diff --git a/daemon/connect/WebDownloader.cpp b/daemon/connect/WebDownloader.cpp index 0a7b5861..52a0d7b2 100644 --- a/daemon/connect/WebDownloader.cpp +++ b/daemon/connect/WebDownloader.cpp @@ -639,13 +639,12 @@ void WebDownloader::Stop() { debug("Trying to stop WebDownloader"); Thread::Stop(); - m_connectionMutex.Lock(); + Guard guard(m_connectionMutex); if (m_connection) { m_connection->SetSuppressErrors(true); m_connection->Cancel(); } - m_connectionMutex.Unlock(); debug("WebDownloader stopped successfully"); } @@ -669,12 +668,11 @@ void WebDownloader::FreeConnection() if (m_connection) { debug("Releasing connection"); - m_connectionMutex.Lock(); + Guard guard(m_connectionMutex); if (m_connection->GetStatus() == Connection::csCancelled) { m_connection->Disconnect(); } m_connection.reset(); - m_connectionMutex.Unlock(); } } diff --git a/daemon/extension/QueueScript.cpp b/daemon/extension/QueueScript.cpp index ad7efc44..94243882 100644 --- a/daemon/extension/QueueScript.cpp +++ b/daemon/extension/QueueScript.cpp @@ -254,7 +254,7 @@ void QueueScriptCoordinator::EnqueueScript(NzbInfo* nzbInfo, EEvent event) return; } - m_queueMutex.Lock(); + Guard guard(m_queueMutex); if (event == qeNzbDownloaded) { @@ -274,7 +274,6 @@ void QueueScriptCoordinator::EnqueueScript(NzbInfo* nzbInfo, EEvent event) (g_Options->GetEventInterval() > 0 && curTime - nzbInfo->GetQueueScriptTime() > 0 && (int)(curTime - nzbInfo->GetQueueScriptTime()) < g_Options->GetEventInterval()))) { - m_queueMutex.Unlock(); return; } @@ -313,8 +312,6 @@ void QueueScriptCoordinator::EnqueueScript(NzbInfo* nzbInfo, EEvent event) nzbInfo->SetQueueScriptTime(Util::CurrentTime()); } } - - m_queueMutex.Unlock(); } bool QueueScriptCoordinator::UsableScript(ScriptConfig::Script& script, NzbInfo* nzbInfo, EEvent event) @@ -418,7 +415,7 @@ void QueueScriptCoordinator::CheckQueue() m_curItem.reset(); DownloadQueue* downloadQueue = DownloadQueue::Lock(); - m_queueMutex.Lock(); + Guard guard(m_queueMutex); NzbInfo* curNzbInfo = nullptr; Queue::iterator itCurItem = m_queue.end(); @@ -454,13 +451,14 @@ void QueueScriptCoordinator::CheckQueue() QueueScriptController::StartScript(curNzbInfo, m_curItem->GetScript(), m_curItem->GetEvent()); } - m_queueMutex.Unlock(); + guard.Release(); DownloadQueue::Unlock(); } bool QueueScriptCoordinator::HasJob(int nzbId, bool* active) { - m_queueMutex.Lock(); + Guard guard(m_queueMutex); + bool working = m_curItem && m_curItem->GetNzbId() == nzbId; if (active) { @@ -477,20 +475,19 @@ bool QueueScriptCoordinator::HasJob(int nzbId, bool* active) } } } - m_queueMutex.Unlock(); return working; } int QueueScriptCoordinator::GetQueueSize() { - m_queueMutex.Lock(); + Guard guard(m_queueMutex); + int queuedCount = m_queue.size(); if (m_curItem) { queuedCount++; } - m_queueMutex.Unlock(); return queuedCount; } diff --git a/daemon/feed/FeedCoordinator.cpp b/daemon/feed/FeedCoordinator.cpp index b0017ce1..395c9e46 100644 --- a/daemon/feed/FeedCoordinator.cpp +++ b/daemon/feed/FeedCoordinator.cpp @@ -93,9 +93,8 @@ void FeedCoordinator::Run() if (g_Options->GetServerMode() && g_Options->GetSaveQueue() && g_Options->GetReloadQueue()) { - m_downloadsMutex.Lock(); + Guard guard(m_downloadsMutex); g_DiskState->LoadFeeds(&m_feeds, &m_feedHistory); - m_downloadsMutex.Unlock(); } int sleepInterval = 100; @@ -113,7 +112,8 @@ void FeedCoordinator::Run() if (!g_Options->GetPauseDownload() || m_force || g_Options->GetUrlForce()) { - m_downloadsMutex.Lock(); + Guard guard(m_downloadsMutex); + time_t current = Util::CurrentTime(); if ((int)m_activeDownloads.size() < g_Options->GetUrlConnections()) { @@ -135,7 +135,6 @@ void FeedCoordinator::Run() } } } - m_downloadsMutex.Unlock(); } CheckSaveFeeds(); @@ -176,12 +175,11 @@ void FeedCoordinator::Stop() Thread::Stop(); debug("Stopping UrlDownloads"); - m_downloadsMutex.Lock(); + Guard guard(m_downloadsMutex); for (FeedDownloader* feedDownloader : m_activeDownloads) { feedDownloader->Stop(); } - m_downloadsMutex.Unlock(); debug("UrlDownloads are notified"); } @@ -193,7 +191,7 @@ void FeedCoordinator::ResetHangingDownloads() return; } - m_downloadsMutex.Lock(); + Guard guard(m_downloadsMutex); time_t tm = Util::CurrentTime(); m_activeDownloads.erase(std::remove_if(m_activeDownloads.begin(), m_activeDownloads.end(), @@ -221,21 +219,18 @@ void FeedCoordinator::ResetHangingDownloads() return false; }), m_activeDownloads.end()); - - m_downloadsMutex.Unlock(); } void FeedCoordinator::LogDebugInfo() { info(" ---------- FeedCoordinator"); - m_downloadsMutex.Lock(); + Guard guard(m_downloadsMutex); info(" Active Downloads: %i", (int)m_activeDownloads.size()); for (FeedDownloader* feedDownloader : m_activeDownloads) { feedDownloader->LogDebugInfo(); } - m_downloadsMutex.Unlock(); } void FeedCoordinator::StartFeedDownload(FeedInfo* feedInfo, bool force) @@ -301,9 +296,10 @@ void FeedCoordinator::FeedCompleted(FeedDownloader* feedDownloader) } // remove downloader from downloader list - m_downloadsMutex.Lock(); - m_activeDownloads.erase(std::find(m_activeDownloads.begin(), m_activeDownloads.end(), feedDownloader)); - m_downloadsMutex.Unlock(); + { + Guard guard(m_downloadsMutex); + m_activeDownloads.erase(std::find(m_activeDownloads.begin(), m_activeDownloads.end(), feedDownloader)); + } if (statusOK) { @@ -319,17 +315,18 @@ void FeedCoordinator::FeedCompleted(FeedDownloader* feedDownloader) std::vector> addedNzbs; - m_downloadsMutex.Lock(); - if (parsed) { - std::unique_ptr feedItems = feedFile->DetachFeedItems(); - addedNzbs = ProcessFeed(feedInfo, feedItems.get()); - feedFile.reset(); + Guard guard(m_downloadsMutex); + if (parsed) + { + std::unique_ptr feedItems = feedFile->DetachFeedItems(); + addedNzbs = ProcessFeed(feedInfo, feedItems.get()); + feedFile.reset(); + } + feedInfo->SetLastUpdate(Util::CurrentTime()); + feedInfo->SetForce(false); + m_save = true; } - feedInfo->SetLastUpdate(Util::CurrentTime()); - feedInfo->SetForce(false); - m_save = true; - m_downloadsMutex.Unlock(); DownloadQueue* downloadQueue = DownloadQueue::Lock(); for (std::unique_ptr& nzbInfo : addedNzbs) @@ -489,7 +486,7 @@ std::shared_ptr FeedCoordinator::PreviewFeed(int id, bool hasCache = false; if (cacheTimeSec > 0 && *cacheId != '\0') { - m_downloadsMutex.Lock(); + Guard guard(m_downloadsMutex); for (FeedCacheItem& feedCacheItem : m_feedCache) { if (!strcmp(feedCacheItem.GetCacheId(), cacheId)) @@ -500,27 +497,28 @@ std::shared_ptr FeedCoordinator::PreviewFeed(int id, break; } } - m_downloadsMutex.Unlock(); } if (!hasCache) { - m_downloadsMutex.Lock(); - bool firstFetch = true; - for (FeedInfo* feedInfo2 : &m_feeds) - { - if (!strcmp(feedInfo2->GetUrl(), feedInfo->GetUrl()) && - !strcmp(feedInfo2->GetFilter(), feedInfo->GetFilter()) && - feedInfo2->GetLastUpdate() > 0) - { - firstFetch = false; - break; - } - } - StartFeedDownload(feedInfo.get(), true); - m_downloadsMutex.Unlock(); + { + Guard guard(m_downloadsMutex); + + for (FeedInfo* feedInfo2 : &m_feeds) + { + if (!strcmp(feedInfo2->GetUrl(), feedInfo->GetUrl()) && + !strcmp(feedInfo2->GetFilter(), feedInfo->GetFilter()) && + feedInfo2->GetLastUpdate() > 0) + { + firstFetch = false; + break; + } + } + + StartFeedDownload(feedInfo.get(), true); + } // wait until the download in a separate thread completes while (feedInfo->GetStatus() == FeedInfo::fsRunning) @@ -567,9 +565,8 @@ std::shared_ptr FeedCoordinator::PreviewFeed(int id, if (cacheTimeSec > 0 && *cacheId != '\0' && !hasCache) { - m_downloadsMutex.Lock(); + Guard guard(m_downloadsMutex); m_feedCache.emplace_back(url, cacheTimeSec, cacheId, Util::CurrentTime(), feedItems); - m_downloadsMutex.Unlock(); } return feedItems; @@ -579,7 +576,7 @@ void FeedCoordinator::FetchFeed(int id) { debug("FetchFeeds"); - m_downloadsMutex.Lock(); + Guard guard(m_downloadsMutex); for (FeedInfo* feedInfo : &m_feeds) { if (feedInfo->GetId() == id || id == 0) @@ -588,7 +585,6 @@ void FeedCoordinator::FetchFeed(int id) m_force = true; } } - m_downloadsMutex.Unlock(); } void FeedCoordinator::DownloadQueueUpdate(Subject* caller, void* aspect) @@ -598,7 +594,7 @@ void FeedCoordinator::DownloadQueueUpdate(Subject* caller, void* aspect) DownloadQueue::Aspect* queueAspect = (DownloadQueue::Aspect*)aspect; if (queueAspect->action == DownloadQueue::eaUrlCompleted) { - m_downloadsMutex.Lock(); + Guard guard(m_downloadsMutex); FeedHistoryInfo* feedHistoryInfo = m_feedHistory.Find(queueAspect->nzbInfo->GetUrl()); if (feedHistoryInfo) { @@ -609,22 +605,19 @@ void FeedCoordinator::DownloadQueueUpdate(Subject* caller, void* aspect) m_feedHistory.emplace_back(queueAspect->nzbInfo->GetUrl(), FeedHistoryInfo::hsFetched, Util::CurrentTime()); } m_save = true; - m_downloadsMutex.Unlock(); } } bool FeedCoordinator::HasActiveDownloads() { - m_downloadsMutex.Lock(); - bool active = !m_activeDownloads.empty(); - m_downloadsMutex.Unlock(); - return active; + Guard guard(m_downloadsMutex); + return !m_activeDownloads.empty(); } void FeedCoordinator::CheckSaveFeeds() { debug("CheckSaveFeeds"); - m_downloadsMutex.Lock(); + Guard guard(m_downloadsMutex); if (m_save) { if (g_Options->GetSaveQueue() && g_Options->GetServerMode()) @@ -633,14 +626,13 @@ void FeedCoordinator::CheckSaveFeeds() } m_save = false; } - m_downloadsMutex.Unlock(); } void FeedCoordinator::CleanupHistory() { debug("CleanupHistory"); - m_downloadsMutex.Lock(); + Guard guard(m_downloadsMutex); time_t oldestUpdate = Util::CurrentTime(); @@ -666,15 +658,13 @@ void FeedCoordinator::CleanupHistory() return false; }), m_feedHistory.end()); - - m_downloadsMutex.Unlock(); } void FeedCoordinator::CleanupCache() { debug("CleanupCache"); - m_downloadsMutex.Lock(); + Guard guard(m_downloadsMutex); time_t curTime = Util::CurrentTime(); @@ -689,6 +679,4 @@ void FeedCoordinator::CleanupCache() } return false; }); - - m_downloadsMutex.Unlock(); } diff --git a/daemon/main/Maintenance.cpp b/daemon/main/Maintenance.cpp index 0de31645..50902683 100644 --- a/daemon/main/Maintenance.cpp +++ b/daemon/main/Maintenance.cpp @@ -54,11 +54,11 @@ private: Maintenance::~Maintenance() { - m_controllerMutex.Lock(); + Guard guard(m_controllerMutex); if (m_updateScriptController) { m_updateScriptController->Detach(); - m_controllerMutex.Unlock(); + guard.Release(); while (m_updateScriptController) { usleep(20*1000); @@ -68,9 +68,8 @@ Maintenance::~Maintenance() void Maintenance::ResetUpdateController() { - m_controllerMutex.Lock(); + Guard guard(m_controllerMutex); m_updateScriptController = nullptr; - m_controllerMutex.Unlock(); } MessageList* Maintenance::LockMessages() @@ -98,9 +97,9 @@ void Maintenance::AddMessage(Message::EKind kind, time_t time, const char * text bool Maintenance::StartUpdate(EBranch branch) { - m_controllerMutex.Lock(); + Guard guard(m_controllerMutex); bool alreadyUpdating = m_updateScriptController != nullptr; - m_controllerMutex.Unlock(); + guard.Release(); if (alreadyUpdating) { diff --git a/daemon/main/Scheduler.cpp b/daemon/main/Scheduler.cpp index b3ce2795..64c7ac63 100644 --- a/daemon/main/Scheduler.cpp +++ b/daemon/main/Scheduler.cpp @@ -30,23 +30,22 @@ void Scheduler::AddTask(std::unique_ptr task) { - m_taskListMutex.Lock(); + Guard guard(m_taskListMutex); m_taskList.push_back(std::move(task)); - m_taskListMutex.Unlock(); } void Scheduler::FirstCheck() { - m_taskListMutex.Lock(); + { + Guard guard(m_taskListMutex); - std::sort(m_taskList.begin(), m_taskList.end(), - [](std::unique_ptr& task1, std::unique_ptr& task2) + std::sort(m_taskList.begin(), m_taskList.end(), + [](std::unique_ptr& task1, std::unique_ptr& task2) { return (task1->m_hours < task2->m_hours) || ((task1->m_hours == task2->m_hours) && (task1->m_minutes < task2->m_minutes)); }); - - m_taskListMutex.Unlock(); + } // check all tasks for the last week CheckTasks(); @@ -75,7 +74,7 @@ void Scheduler::CheckTasks() { PrepareLog(); - m_taskListMutex.Lock(); + Guard guard(m_taskListMutex); time_t current = Util::CurrentTime(); @@ -151,7 +150,7 @@ void Scheduler::CheckTasks() m_lastCheck = current; - m_taskListMutex.Unlock(); + guard.Release(); PrintLog(); } diff --git a/daemon/nntp/ArticleDownloader.cpp b/daemon/nntp/ArticleDownloader.cpp index 79557507..e8e09bc9 100644 --- a/daemon/nntp/ArticleDownloader.cpp +++ b/daemon/nntp/ArticleDownloader.cpp @@ -627,13 +627,12 @@ void ArticleDownloader::Stop() { debug("Trying to stop ArticleDownloader"); Thread::Stop(); - m_connectionMutex.Lock(); + Guard guard(m_connectionMutex); if (m_connection) { m_connection->SetSuppressErrors(true); m_connection->Cancel(); } - m_connectionMutex.Unlock(); debug("ArticleDownloader stopped successfully"); } @@ -658,7 +657,7 @@ void ArticleDownloader::FreeConnection(bool keepConnected) if (m_connection) { debug("Releasing connection"); - m_connectionMutex.Lock(); + Guard guard(m_connectionMutex); if (!keepConnected || m_connection->GetStatus() == Connection::csCancelled) { m_connection->Disconnect(); @@ -666,7 +665,6 @@ void ArticleDownloader::FreeConnection(bool keepConnected) AddServerData(); g_ServerPool->FreeConnection(m_connection, true); m_connection = nullptr; - m_connectionMutex.Unlock(); } } diff --git a/daemon/nntp/ArticleWriter.cpp b/daemon/nntp/ArticleWriter.cpp index 44a71fd1..f8c9aed0 100644 --- a/daemon/nntp/ArticleWriter.cpp +++ b/daemon/nntp/ArticleWriter.cpp @@ -795,34 +795,36 @@ ArticleCache::ArticleCache() CachedSegmentData ArticleCache::Alloc(int size) { - m_allocMutex.Lock(); void* p = nullptr; - if (m_allocated + size <= (size_t)g_Options->GetArticleCache() * 1024 * 1024) + { - p = malloc(size); - if (p) + Guard guard(m_allocMutex); + if (m_allocated + size <= (size_t)g_Options->GetArticleCache() * 1024 * 1024) { - if (!m_allocated && g_Options->GetSaveQueue() && g_Options->GetServerMode() && g_Options->GetContinuePartial()) + p = malloc(size); + if (p) { - g_DiskState->WriteCacheFlag(); + if (!m_allocated && g_Options->GetSaveQueue() && g_Options->GetServerMode() && g_Options->GetContinuePartial()) + { + g_DiskState->WriteCacheFlag(); + } + m_allocated += size; } - m_allocated += size; } } - m_allocMutex.Unlock(); return CachedSegmentData((char*)p, size); } bool ArticleCache::Realloc(CachedSegmentData* segment, int newSize) { - m_allocMutex.Lock(); + Guard guard(m_allocMutex); + void* p = realloc(segment->m_data, newSize); if (p) { m_allocated += newSize - segment->m_size; } - m_allocMutex.Unlock(); return p; } @@ -833,13 +835,12 @@ void ArticleCache::Free(CachedSegmentData* segment) { free(segment->m_data); - m_allocMutex.Lock(); + Guard guard(m_allocMutex); m_allocated -= segment->m_size; if (!m_allocated && g_Options->GetSaveQueue() && g_Options->GetServerMode() && g_Options->GetContinuePartial()) { g_DiskState->DeleteCacheFlag(); } - m_allocMutex.Unlock(); } } diff --git a/daemon/nntp/ServerPool.cpp b/daemon/nntp/ServerPool.cpp index 7f81298d..e2162523 100644 --- a/daemon/nntp/ServerPool.cpp +++ b/daemon/nntp/ServerPool.cpp @@ -93,7 +93,7 @@ void ServerPool::InitConnections() { debug("Initializing connections in ServerPool"); - m_connectionsMutex.Lock(); + Guard guard(m_connectionsMutex); NormalizeLevels(); m_levels.clear(); @@ -135,14 +135,12 @@ void ServerPool::InitConnections() } m_generation++; - - m_connectionsMutex.Unlock(); } NntpConnection* ServerPool::GetConnection(int level, NewsServer* wantServer, RawServerList* ignoreServers) { PooledConnection* connection = nullptr; - m_connectionsMutex.Lock(); + Guard guard(m_connectionsMutex); time_t curTime = Util::CurrentTime(); @@ -204,8 +202,6 @@ NntpConnection* ServerPool::GetConnection(int level, NewsServer* wantServer, Raw } } - m_connectionsMutex.Unlock(); - return connection; } @@ -216,7 +212,7 @@ void ServerPool::FreeConnection(NntpConnection* connection, bool used) debug("Freeing used connection"); } - m_connectionsMutex.Lock(); + Guard guard(m_connectionsMutex); ((PooledConnection*)connection)->SetInUse(false); if (used) @@ -228,17 +224,15 @@ void ServerPool::FreeConnection(NntpConnection* connection, bool used) { m_levels[connection->GetNewsServer()->GetNormLevel()]++; } - - m_connectionsMutex.Unlock(); } void ServerPool::BlockServer(NewsServer* newsServer) { - m_connectionsMutex.Lock(); + Guard guard(m_connectionsMutex); time_t curTime = Util::CurrentTime(); bool newBlock = newsServer->GetBlockTime() != curTime; newsServer->SetBlockTime(curTime); - m_connectionsMutex.Unlock(); + guard.Release(); if (newBlock && m_retryInterval > 0) { @@ -248,7 +242,7 @@ void ServerPool::BlockServer(NewsServer* newsServer) void ServerPool::CloseUnusedConnections() { - m_connectionsMutex.Lock(); + Guard guard(m_connectionsMutex); time_t curtime = Util::CurrentTime(); @@ -312,8 +306,6 @@ void ServerPool::CloseUnusedConnections() } } } - - m_connectionsMutex.Unlock(); } void ServerPool::Changed() @@ -330,7 +322,7 @@ void ServerPool::LogDebugInfo() info(" Max-Level: %i", m_maxNormLevel); - m_connectionsMutex.Lock(); + Guard guard(m_connectionsMutex); time_t curTime = Util::CurrentTime(); @@ -359,6 +351,4 @@ void ServerPool::LogDebugInfo() connection->GetNewsServer()->GetLevel(), connection->GetNewsServer()->GetNormLevel(), (int)connection->GetInUse()); } - - m_connectionsMutex.Unlock(); } diff --git a/daemon/nntp/StatMeter.cpp b/daemon/nntp/StatMeter.cpp index 26ed077e..72a4c300 100644 --- a/daemon/nntp/StatMeter.cpp +++ b/daemon/nntp/StatMeter.cpp @@ -232,7 +232,7 @@ void StatMeter::IntervalCheck() void StatMeter::EnterLeaveStandBy(bool enter) { - m_statMutex.Lock(); + Guard guard(m_statMutex); m_standBy = enter; if (enter) { @@ -251,12 +251,11 @@ void StatMeter::EnterLeaveStandBy(bool enter) m_pausedFrom = 0; ResetSpeedStat(); } - m_statMutex.Unlock(); } void StatMeter::CalcTotalStat(int* upTimeSec, int* dnTimeSec, int64* allBytes, bool* standBy) { - m_statMutex.Lock(); + Guard guard(m_statMutex); if (m_startServer > 0) { *upTimeSec = (int)(Util::CurrentTime() - m_startServer); @@ -275,7 +274,6 @@ void StatMeter::CalcTotalStat(int* upTimeSec, int* dnTimeSec, int64* allBytes, b *dnTimeSec = (int)(Util::CurrentTime() - m_startDownload); } *allBytes = m_allBytes; - m_statMutex.Unlock(); } // Average speed in last 30 seconds @@ -308,10 +306,7 @@ void StatMeter::AddSpeedReading(int bytes) time_t curTime = Util::CurrentTime(); int nowSlot = (int)curTime / SPEEDMETER_SLOTSIZE; - if (g_Options->GetAccurateRate()) - { - m_speedMutex.Lock(); - } + Guard guard(g_Options->GetAccurateRate() ? &m_speedMutex : nullptr); if (curTime != m_curSecTime) { @@ -360,11 +355,6 @@ void StatMeter::AddSpeedReading(int bytes) m_speedBytes[m_speedBytesIndex] += bytes; m_speedTotalBytes += bytes; m_allBytes += bytes; - - if (g_Options->GetAccurateRate()) - { - m_speedMutex.Unlock(); - } } void StatMeter::ResetSpeedStat() @@ -400,7 +390,7 @@ void StatMeter::LogDebugInfo() info(" Bytes[%i]: %i, Time[%i]: %i", i, m_speedBytes[i], i, m_speedTime[i]); } - m_volumeMutex.Lock(); + Guard guard(m_volumeMutex); int index = 0; for (ServerVolume& serverVolume : m_serverVolumes) { @@ -408,7 +398,6 @@ void StatMeter::LogDebugInfo() serverVolume.LogDebugInfo(); index++; } - m_volumeMutex.Unlock(); } void StatMeter::AddServerData(int bytes, int serverId) @@ -418,11 +407,10 @@ void StatMeter::AddServerData(int bytes, int serverId) return; } - m_volumeMutex.Lock(); + Guard guard(m_volumeMutex); m_serverVolumes[0].AddData(bytes); m_serverVolumes[serverId].AddData(bytes); m_statChanged = true; - m_volumeMutex.Unlock(); } ServerVolumes* StatMeter::LockServerVolumes() @@ -450,15 +438,14 @@ void StatMeter::Save() return; } - m_volumeMutex.Lock(); + Guard guard(m_volumeMutex); g_DiskState->SaveStats(g_ServerPool->GetServers(), &m_serverVolumes); m_statChanged = false; - m_volumeMutex.Unlock(); } bool StatMeter::Load(bool* perfectServerMatch) { - m_volumeMutex.Lock(); + Guard guard(m_volumeMutex); bool ok = g_DiskState->LoadStats(g_ServerPool->GetServers(), &m_serverVolumes, perfectServerMatch); @@ -467,7 +454,5 @@ bool StatMeter::Load(bool* perfectServerMatch) serverVolume.CalcSlots(serverVolume.GetDataTime() + g_Options->GetLocalTimeOffset()); } - m_volumeMutex.Unlock(); - return ok; } diff --git a/daemon/postprocess/ParChecker.cpp b/daemon/postprocess/ParChecker.cpp index 52ee89b9..1d3e6f90 100644 --- a/daemon/postprocess/ParChecker.cpp +++ b/daemon/postprocess/ParChecker.cpp @@ -281,11 +281,11 @@ void Repairer::RepairBlock(Par2::u32 inputindex, Par2::u32 outputindex, size_t b if (noiselevel > Par2::CommandLine::nlQuiet) { // Update a progress indicator - progresslock.Lock(); + Guard guard(progresslock); Par2::u32 oldfraction = (Par2::u32)(1000 * progress / totaldata); progress += blocklength; Par2::u32 newfraction = (Par2::u32)(1000 * progress / totaldata); - progresslock.Unlock(); + guard.Release(); if (oldfraction != newfraction) { @@ -648,10 +648,12 @@ bool ParChecker::LoadMainParBak() { while (!IsStopped()) { - m_queuedParFilesMutex.Lock(); - bool hasMorePars = !m_queuedParFiles.empty(); - m_queuedParFiles.clear(); - m_queuedParFilesMutex.Unlock(); + bool hasMorePars = false; + { + Guard guard(m_queuedParFilesMutex); + hasMorePars = !m_queuedParFiles.empty(); + m_queuedParFiles.clear(); + } if (hasMorePars) { @@ -667,10 +669,11 @@ bool ParChecker::LoadMainParBak() UpdateProgress(); } - m_queuedParFilesMutex.Lock(); - hasMorePars = !m_queuedParFiles.empty(); - m_queuedParFilesChanged = false; - m_queuedParFilesMutex.Unlock(); + { + Guard guard(m_queuedParFilesMutex); + hasMorePars = !m_queuedParFiles.empty(); + m_queuedParFilesChanged = false; + } if (!requested && !hasMorePars) { @@ -683,9 +686,10 @@ bool ParChecker::LoadMainParBak() bool queuedParFilesChanged = false; while (!queuedParFilesChanged && !IsStopped() && !m_cancelled) { - m_queuedParFilesMutex.Lock(); - queuedParFilesChanged = m_queuedParFilesChanged; - m_queuedParFilesMutex.Unlock(); + { + Guard guard(m_queuedParFilesMutex); + queuedParFilesChanged = m_queuedParFilesChanged; + } usleep(100 * 1000); } } @@ -713,9 +717,11 @@ int ParChecker::ProcessMorePars() PrintMessage(Message::mkInfo, "Need more %i par-block(s) for %s", missingblockcount, *m_infoName); } - m_queuedParFilesMutex.Lock(); - bool hasMorePars = !m_queuedParFiles.empty(); - m_queuedParFilesMutex.Unlock(); + bool hasMorePars = false; + { + Guard guard(m_queuedParFilesMutex); + hasMorePars = !m_queuedParFiles.empty(); + } if (!hasMorePars) { @@ -728,10 +734,11 @@ int ParChecker::ProcessMorePars() UpdateProgress(); } - m_queuedParFilesMutex.Lock(); - hasMorePars = !m_queuedParFiles.empty(); - m_queuedParFilesChanged = false; - m_queuedParFilesMutex.Unlock(); + { + Guard guard(m_queuedParFilesMutex); + hasMorePars = !m_queuedParFiles.empty(); + m_queuedParFilesChanged = false; + } if (!requested && !hasMorePars) { @@ -745,9 +752,10 @@ int ParChecker::ProcessMorePars() bool queuedParFilesChanged = false; while (!queuedParFilesChanged && !IsStopped() && !m_cancelled) { - m_queuedParFilesMutex.Lock(); - queuedParFilesChanged = m_queuedParFilesChanged; - m_queuedParFilesMutex.Unlock(); + { + Guard guard(m_queuedParFilesMutex); + queuedParFilesChanged = m_queuedParFilesChanged; + } usleep(100 * 1000); } } @@ -771,10 +779,10 @@ int ParChecker::ProcessMorePars() bool ParChecker::LoadMorePars() { - m_queuedParFilesMutex.Lock(); + Guard guard(m_queuedParFilesMutex); FileList moreFiles = std::move(m_queuedParFiles); m_queuedParFiles.clear(); - m_queuedParFilesMutex.Unlock(); + guard.Release(); for (CString& parFilename : moreFiles) { @@ -794,17 +802,15 @@ bool ParChecker::LoadMorePars() void ParChecker::AddParFile(const char * parFilename) { - m_queuedParFilesMutex.Lock(); + Guard guard(m_queuedParFilesMutex); m_queuedParFiles.push_back(parFilename); m_queuedParFilesChanged = true; - m_queuedParFilesMutex.Unlock(); } void ParChecker::QueueChanged() { - m_queuedParFilesMutex.Lock(); + Guard guard(m_queuedParFilesMutex); m_queuedParFilesChanged = true; - m_queuedParFilesMutex.Unlock(); } bool ParChecker::AddSplittedFragments() diff --git a/daemon/queue/DownloadInfo.cpp b/daemon/queue/DownloadInfo.cpp index 3a1909b9..10a0b5cb 100644 --- a/daemon/queue/DownloadInfo.cpp +++ b/daemon/queue/DownloadInfo.cpp @@ -385,7 +385,8 @@ void NzbInfo::AddMessage(Message::EKind kind, const char * text) break; } - m_logMutex.Lock(); + Guard guard(m_logMutex); + m_messages.emplace_back(++m_idMessageGen, kind, Util::CurrentTime(), text); if (g_Options->GetSaveQueue() && g_Options->GetServerMode() && g_Options->GetNzbLog()) @@ -400,7 +401,6 @@ void NzbInfo::AddMessage(Message::EKind kind, const char * text) } m_cachedMessageCount = m_messages.size(); - m_logMutex.Unlock(); } void NzbInfo::PrintMessage(Message::EKind kind, const char* format, ...) @@ -418,10 +418,9 @@ void NzbInfo::PrintMessage(Message::EKind kind, const char* format, ...) void NzbInfo::ClearMessages() { - m_logMutex.Lock(); + Guard guard(m_logMutex); m_messages.clear(); m_cachedMessageCount = 0; - m_logMutex.Unlock(); } void NzbInfo::MoveFileList(NzbInfo* srcNzbInfo) diff --git a/daemon/queue/Scanner.cpp b/daemon/queue/Scanner.cpp index 0819eeda..7bbfcbf3 100644 --- a/daemon/queue/Scanner.cpp +++ b/daemon/queue/Scanner.cpp @@ -83,7 +83,7 @@ void Scanner::ServiceWork() return; } - m_scanMutex.Lock(); + Guard guard(m_scanMutex); if (m_requestedNzbDirScan || (!g_Options->GetPauseScan() && g_Options->GetNzbDirInterval() > 0 && @@ -126,8 +126,6 @@ void Scanner::ServiceWork() m_queueList.clear(); } m_nzbDirInterval += 200; - - m_scanMutex.Unlock(); } /** @@ -463,10 +461,11 @@ bool Scanner::AddFileToQueue(const char* filename, const char* nzbName, const ch void Scanner::ScanNzbDir(bool syncMode) { - m_scanMutex.Lock(); - m_scanning = true; - m_requestedNzbDirScan = true; - m_scanMutex.Unlock(); + { + Guard guard(m_scanMutex); + m_scanning = true; + m_requestedNzbDirScan = true; + } while (syncMode && (m_scanning || m_requestedNzbDirScan)) { @@ -542,14 +541,13 @@ Scanner::EAddStatus Scanner::AddExternalFile(const char* nzbName, const char* ca num++; } - m_scanMutex.Lock(); + Guard guard(m_scanMutex); if (!FileSystem::MoveFile(tempFileName, scanFileName)) { error("Could not move file %s to %s: %s", *tempFileName, *scanFileName, *FileSystem::GetLastErrorMessage()); FileSystem::DeleteFile(tempFileName); - m_scanMutex.Unlock(); // UNLOCK return asFailed; } @@ -566,7 +564,7 @@ Scanner::EAddStatus Scanner::AddExternalFile(const char* nzbName, const char* ca dupeKey, dupeScore, dupeMode, parameters, addTop, addPaused, urlInfo, &addStatus, nzbId); - m_scanMutex.Unlock(); + guard.Release(); ScanNzbDir(true); diff --git a/daemon/util/Log.cpp b/daemon/util/Log.cpp index b9496250..aa7289e8 100644 --- a/daemon/util/Log.cpp +++ b/daemon/util/Log.cpp @@ -44,12 +44,11 @@ void Log::LogDebugInfo() info("Dumping debug info to log"); info("--------------------------------------------"); - m_debugMutex.Lock(); + Guard guard(m_debugMutex); for (Debuggable* debuggable : m_debuggables) { debuggable->LogDebugInfo(); } - m_debugMutex.Unlock(); info("--------------------------------------------"); } @@ -134,7 +133,7 @@ void debug(const char* msg, ...) tmp2.Format("%s", tmp1); #endif - g_Log->m_logMutex.Lock(); + Guard guard(g_Log->m_logMutex); if (!g_Options && g_Log->m_extraDebug) { @@ -150,8 +149,6 @@ void debug(const char* msg, ...) { g_Log->Filelog("DEBUG\t%s", *tmp2); } - - g_Log->m_logMutex.Unlock(); } #endif @@ -165,7 +162,7 @@ void error(const char* msg, ...) tmp2[1024-1] = '\0'; va_end(ap); - g_Log->m_logMutex.Lock(); + Guard guard(g_Log->m_logMutex); Options::EMessageTarget messageTarget = g_Options ? g_Options->GetErrorTarget() : Options::mtBoth; if (messageTarget == Options::mtScreen || messageTarget == Options::mtBoth) @@ -176,8 +173,6 @@ void error(const char* msg, ...) { g_Log->Filelog("ERROR\t%s", tmp2); } - - g_Log->m_logMutex.Unlock(); } void warn(const char* msg, ...) @@ -190,7 +185,7 @@ void warn(const char* msg, ...) tmp2[1024-1] = '\0'; va_end(ap); - g_Log->m_logMutex.Lock(); + Guard guard(g_Log->m_logMutex); Options::EMessageTarget messageTarget = g_Options ? g_Options->GetWarningTarget() : Options::mtScreen; if (messageTarget == Options::mtScreen || messageTarget == Options::mtBoth) @@ -201,8 +196,6 @@ void warn(const char* msg, ...) { g_Log->Filelog("WARNING\t%s", tmp2); } - - g_Log->m_logMutex.Unlock(); } void info(const char* msg, ...) @@ -215,7 +208,7 @@ void info(const char* msg, ...) tmp2[1024-1] = '\0'; va_end(ap); - g_Log->m_logMutex.Lock(); + Guard guard(g_Log->m_logMutex); Options::EMessageTarget messageTarget = g_Options ? g_Options->GetInfoTarget() : Options::mtScreen; if (messageTarget == Options::mtScreen || messageTarget == Options::mtBoth) @@ -226,8 +219,6 @@ void info(const char* msg, ...) { g_Log->Filelog("INFO\t%s", tmp2); } - - g_Log->m_logMutex.Unlock(); } void detail(const char* msg, ...) @@ -240,7 +231,7 @@ void detail(const char* msg, ...) tmp2[1024-1] = '\0'; va_end(ap); - g_Log->m_logMutex.Lock(); + Guard guard(g_Log->m_logMutex); Options::EMessageTarget messageTarget = g_Options ? g_Options->GetDetailTarget() : Options::mtScreen; if (messageTarget == Options::mtScreen || messageTarget == Options::mtBoth) @@ -251,16 +242,13 @@ void detail(const char* msg, ...) { g_Log->Filelog("DETAIL\t%s", tmp2); } - - g_Log->m_logMutex.Unlock(); } void Log::Clear() { - m_logMutex.Lock(); + Guard guard(m_logMutex); m_messages.clear(); - m_logMutex.Unlock(); } void Log::AddMessage(Message::EKind kind, const char * text) @@ -419,14 +407,12 @@ void Log::InitOptions() void Log::RegisterDebuggable(Debuggable* debuggable) { - m_debugMutex.Lock(); + Guard guard(m_debugMutex); m_debuggables.push_back(debuggable); - m_debugMutex.Unlock(); } void Log::UnregisterDebuggable(Debuggable* debuggable) { - m_debugMutex.Lock(); + Guard guard(m_debugMutex); m_debuggables.remove(debuggable); - m_debugMutex.Unlock(); } diff --git a/daemon/util/Script.cpp b/daemon/util/Script.cpp index 5e1a23db..8aac89ad 100644 --- a/daemon/util/Script.cpp +++ b/daemon/util/Script.cpp @@ -153,9 +153,8 @@ ScriptController::ScriptController() { ResetEnv(); - m_runningMutex.Lock(); + Guard guard(m_runningMutex); m_runningScripts.push_back(this); - m_runningMutex.Unlock(); } ScriptController::~ScriptController() @@ -165,9 +164,8 @@ ScriptController::~ScriptController() void ScriptController::UnregisterRunningScript() { - m_runningMutex.Lock(); + Guard guard(m_runningMutex); m_runningScripts.erase(std::remove(m_runningScripts.begin(), m_runningScripts.end(), this), m_runningScripts.end()); - m_runningMutex.Unlock(); } void ScriptController::ResetEnv() @@ -641,7 +639,7 @@ void ScriptController::Terminate() void ScriptController::TerminateAll() { - m_runningMutex.Lock(); + Guard guard(m_runningMutex); for (ScriptController* script : m_runningScripts) { if (script->m_processId && !script->m_detached) @@ -649,7 +647,6 @@ void ScriptController::TerminateAll() script->Terminate(); } } - m_runningMutex.Unlock(); } void ScriptController::Detach() diff --git a/daemon/util/Thread.cpp b/daemon/util/Thread.cpp index 1cb4eb0d..98e4366e 100644 --- a/daemon/util/Thread.cpp +++ b/daemon/util/Thread.cpp @@ -102,7 +102,7 @@ void Thread::Start() // We lock mutex m_pMutexThread on calling pthread_create; the started thread // then also try to lock the mutex (see thread_handler) and therefore // must wait until we unlock it - m_threadMutex->Lock(); + Guard guard(m_threadMutex); #ifdef WIN32 m_threadObj = (HANDLE)_beginthread(Thread::thread_handler, 0, (void*)this); @@ -111,12 +111,10 @@ void Thread::Start() pthread_attr_t m_attr; pthread_attr_init(&m_attr); pthread_attr_setdetachstate(&m_attr, PTHREAD_CREATE_DETACHED); - pthread_attr_setinheritsched(&m_attr , PTHREAD_INHERIT_SCHED); + pthread_attr_setinheritsched(&m_attr, PTHREAD_INHERIT_SCHED); m_running = !pthread_create(&m_threadObj, &m_attr, Thread::thread_handler, (void *) this); pthread_attr_destroy(&m_attr); #endif - - m_threadMutex->Unlock(); } void Thread::Stop() @@ -137,7 +135,7 @@ bool Thread::Kill() { debug("Killing Thread"); - m_threadMutex->Lock(); + Guard guard(m_threadMutex); #ifdef WIN32 bool terminated = TerminateThread(m_threadObj, 0) != 0; @@ -149,7 +147,6 @@ bool Thread::Kill() { m_threadCount--; } - m_threadMutex->Unlock(); return terminated; } @@ -159,9 +156,10 @@ void __cdecl Thread::thread_handler(void* object) void* Thread::thread_handler(void* object) #endif { - m_threadMutex->Lock(); - m_threadCount++; - m_threadMutex->Unlock(); + { + Guard guard(m_threadMutex); + m_threadCount++; + } debug("Entering Thread-func"); @@ -179,9 +177,10 @@ void* Thread::thread_handler(void* object) delete thread; } - m_threadMutex->Lock(); - m_threadCount--; - m_threadMutex->Unlock(); + { + Guard guard(m_threadMutex); + m_threadCount--; + } #ifndef WIN32 return nullptr; @@ -190,8 +189,7 @@ void* Thread::thread_handler(void* object) int Thread::GetThreadCount() { - m_threadMutex->Lock(); + Guard guard(m_threadMutex); int threadCount = m_threadCount; - m_threadMutex->Unlock(); return threadCount; } diff --git a/daemon/util/Thread.h b/daemon/util/Thread.h index 8b5ea44e..35bd4632 100644 --- a/daemon/util/Thread.h +++ b/daemon/util/Thread.h @@ -39,6 +39,19 @@ private: #endif }; +class Guard +{ +public: + Guard(Mutex& mutex) : m_mutex(&mutex) { if (m_mutex) m_mutex->Lock(); } + Guard(Mutex* mutex) : m_mutex(mutex) { if (m_mutex) m_mutex->Lock(); } + Guard(std::unique_ptr& mutex) : m_mutex(mutex.get()) { if (m_mutex) m_mutex->Lock(); } + ~Guard() { Release(); } + void Release() { if (m_mutex) { m_mutex->Unlock(); m_mutex = nullptr; } } + +private: + Mutex* m_mutex; +}; + class Thread { public: