fixed #356: crash during download caused by a race condition

nzbget · Mar 31, 2017 · 2579ce6 · 2579ce6
1 parent 77f8698
commit 2579ce6
Show file tree

Hide file tree

Showing 2 changed files with 33 additions and 21 deletions.
diff --git a/daemon/queue/QueueCoordinator.cpp b/daemon/queue/QueueCoordinator.cpp
@@ -568,12 +568,14 @@ void QueueCoordinator::ArticleCompleted(ArticleDownloader* articleDownloader)
 	debug("Article downloaded");
 
 	FileInfo* fileInfo = articleDownloader->GetFileInfo();
-	NzbInfo* nzbInfo = fileInfo->GetNzbInfo();
-	ArticleInfo* articleInfo = articleDownloader->GetArticleInfo();
-	bool retry = false;
-	bool fileCompleted = false;
+	bool completeFileParts = false;
 
 	{
+		NzbInfo* nzbInfo = fileInfo->GetNzbInfo();
+		ArticleInfo* articleInfo = articleDownloader->GetArticleInfo();
+		bool retry = false;
+		bool fileCompleted = false;
+
 		GuardedDownloadQueue downloadQueue = DownloadQueue::Guard();
 
 		if (articleDownloader->GetStatus() == ArticleDownloader::adFinished)
@@ -642,35 +644,44 @@ void QueueCoordinator::ArticleCompleted(ArticleDownloader* articleDownloader)
 		{
 			fileCompleted = true;
 		}
-	}
 
-	bool deleteFileObj = false;
+		completeFileParts = fileCompleted && (!fileInfo->GetDeleted() || nzbInfo->GetParking());
 
-	if (fileCompleted && (!fileInfo->GetDeleted() || nzbInfo->GetParking()))
+		if (!completeFileParts)
+		{
+			DeleteDownloader(downloadQueue, articleDownloader, false);
+		}
+	}
+
+	if (completeFileParts)
 	{
 		// all jobs done
 		articleDownloader->CompleteFileParts();
 		fileInfo->SetPartialChanged(false);
-		deleteFileObj = true;
-	}
 
-	{
 		GuardedDownloadQueue downloadQueue = DownloadQueue::Guard();
+		DeleteDownloader(downloadQueue, articleDownloader, true);
+	}
+}
 
-		bool hasOtherDownloaders = fileInfo->GetActiveDownloads() > 1;
-		deleteFileObj |= fileInfo->GetDeleted() && !hasOtherDownloaders;
+void QueueCoordinator::DeleteDownloader(DownloadQueue* downloadQueue,
+	ArticleDownloader* articleDownloader, bool fileCompleted)
+{
+	FileInfo* fileInfo = articleDownloader->GetFileInfo();
+	NzbInfo* nzbInfo = fileInfo->GetNzbInfo();
+	bool hasOtherDownloaders = fileInfo->GetActiveDownloads() > 1;
+	bool deleteFileObj = fileCompleted || (fileInfo->GetDeleted() && !hasOtherDownloaders);
 
-		// remove downloader from downloader list
-		m_activeDownloads.erase(std::find(m_activeDownloads.begin(), m_activeDownloads.end(), articleDownloader));
+	// remove downloader from downloader list
+	m_activeDownloads.erase(std::find(m_activeDownloads.begin(), m_activeDownloads.end(), articleDownloader));
 
-		fileInfo->SetActiveDownloads(fileInfo->GetActiveDownloads() - 1);
-		nzbInfo->SetActiveDownloads(nzbInfo->GetActiveDownloads() - 1);
+	fileInfo->SetActiveDownloads(fileInfo->GetActiveDownloads() - 1);
+	nzbInfo->SetActiveDownloads(nzbInfo->GetActiveDownloads() - 1);
 
-		if (deleteFileObj)
-		{
-			DeleteFileInfo(downloadQueue, fileInfo, fileCompleted);
-			downloadQueue->Save();
-		}
+	if (deleteFileObj)
+	{
+		DeleteFileInfo(downloadQueue, fileInfo, fileCompleted);
+		downloadQueue->Save();
 	}
 }
 

diff --git a/daemon/queue/QueueCoordinator.h b/daemon/queue/QueueCoordinator.h
@@ -83,6 +83,7 @@ class QueueCoordinator : public Thread, public Observer, public Debuggable
 	bool GetNextArticle(DownloadQueue* downloadQueue, FileInfo* &fileInfo, ArticleInfo* &articleInfo);
 	void StartArticleDownload(FileInfo* fileInfo, ArticleInfo* articleInfo, NntpConnection* connection);
 	void ArticleCompleted(ArticleDownloader* articleDownloader);
+	void DeleteDownloader(DownloadQueue* downloadQueue, ArticleDownloader* articleDownloader, bool fileCompleted);
 	void DeleteFileInfo(DownloadQueue* downloadQueue, FileInfo* fileInfo, bool completed);
 	void CheckHealth(DownloadQueue* downloadQueue, FileInfo* fileInfo);
 	void ResetHangingDownloads();