[Curl] Deadlock when downloading.
authorpeavo@outlook.com <peavo@outlook.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 26 Aug 2015 19:01:44 +0000 (19:01 +0000)
committerpeavo@outlook.com <peavo@outlook.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 26 Aug 2015 19:01:44 +0000 (19:01 +0000)
https://bugs.webkit.org/show_bug.cgi?id=148438

Reviewed by Alex Christensen.

A thread should not try locking when it already has got the lock,
this will create a deadlock.

* platform/network/curl/CurlDownload.cpp:
(WebCore::CurlDownloadManager::startThreadIfNeeded):
(WebCore::CurlDownloadManager::stopThread):
(WebCore::CurlDownloadManager::stopThreadIfIdle):
(WebCore::CurlDownload::~CurlDownload):
(WebCore::CurlDownload::moveFileToDestination):
(WebCore::CurlDownload::didFail):
* platform/network/curl/CurlDownload.h:
(WebCore::CurlDownloadManager::getMultiHandle):
(WebCore::CurlDownloadManager::runThread):
(WebCore::CurlDownloadManager::setRunThread):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@188977 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/platform/network/curl/CurlDownload.cpp
Source/WebCore/platform/network/curl/CurlDownload.h

index c802a5e..9f61f58 100644 (file)
@@ -1,3 +1,25 @@
+2015-08-26  Per Arne Vollan  <peavo@outlook.com>
+
+        [Curl] Deadlock when downloading.
+        https://bugs.webkit.org/show_bug.cgi?id=148438
+
+        Reviewed by Alex Christensen.
+
+        A thread should not try locking when it already has got the lock,
+        this will create a deadlock.
+
+        * platform/network/curl/CurlDownload.cpp:
+        (WebCore::CurlDownloadManager::startThreadIfNeeded):
+        (WebCore::CurlDownloadManager::stopThread):
+        (WebCore::CurlDownloadManager::stopThreadIfIdle):
+        (WebCore::CurlDownload::~CurlDownload):
+        (WebCore::CurlDownload::moveFileToDestination):
+        (WebCore::CurlDownload::didFail):
+        * platform/network/curl/CurlDownload.h:
+        (WebCore::CurlDownloadManager::getMultiHandle):
+        (WebCore::CurlDownloadManager::runThread):
+        (WebCore::CurlDownloadManager::setRunThread):
+
 2015-08-26  Jinyoung Hur  <hur.ims@navercorp.com>
 
         [Cairo] Accelerated canvas should fall back to non-accelerated canvas on creation failure
index c6bfb30..809cf08 100644 (file)
@@ -60,9 +60,11 @@ CurlDownloadManager::~CurlDownloadManager()
 
 bool CurlDownloadManager::add(CURL* curlHandle)
 {
-    LockHolder locker(m_mutex);
+    {
+        LockHolder locker(m_mutex);
+        m_pendingHandleList.append(curlHandle);
+    }
 
-    m_pendingHandleList.append(curlHandle);
     startThreadIfNeeded();
 
     return true;
@@ -91,17 +93,17 @@ int CurlDownloadManager::getPendingDownloadCount() const
 
 void CurlDownloadManager::startThreadIfNeeded()
 {
-    if (!m_runThread) {
+    if (!runThread()) {
         if (m_threadId)
             waitForThreadCompletion(m_threadId);
-        m_runThread = true;
+        setRunThread(true);
         m_threadId = createThread(downloadThread, this, "downloadThread");
     }
 }
 
 void CurlDownloadManager::stopThread()
 {
-    m_runThread = false;
+    setRunThread(false);
 
     if (m_threadId) {
         waitForThreadCompletion(m_threadId);
@@ -111,8 +113,6 @@ void CurlDownloadManager::stopThread()
 
 void CurlDownloadManager::stopThreadIfIdle()
 {
-    LockHolder locker(m_mutex);
-
     if (!getActiveDownloadCount() && !getPendingDownloadCount())
         setRunThread(false);
 }
@@ -245,13 +245,15 @@ CurlDownload::CurlDownload()
 
 CurlDownload::~CurlDownload()
 {
-    LockHolder locker(m_mutex);
+    {
+        LockHolder locker(m_mutex);
 
-    if (m_url)
-        fastFree(m_url);
+        if (m_url)
+            fastFree(m_url);
 
-    if (m_customHeaders)
-        curl_slist_free_all(m_customHeaders);
+        if (m_customHeaders)
+            curl_slist_free_all(m_customHeaders);
+    }
 
     closeFile();
     moveFileToDestination();
@@ -345,6 +347,8 @@ void CurlDownload::closeFile()
 
 void CurlDownload::moveFileToDestination()
 {
+    LockHolder locker(m_mutex);
+
     if (m_destination.isEmpty())
         return;
 
@@ -464,10 +468,10 @@ void CurlDownload::didFinish()
 
 void CurlDownload::didFail()
 {
-    LockHolder locker(m_mutex);
-
     closeFile();
 
+    LockHolder locker(m_mutex);
+
     if (m_deletesFileUponFailure)
         deleteFile(m_tempPath);
 
index 6bbc2e6..02ce731 100644 (file)
@@ -61,8 +61,8 @@ private:
 
     CURLM* getMultiHandle() const { return m_curlMultiHandle; }
 
-    bool runThread() const { return m_runThread; }
-    void setRunThread(bool runThread) { m_runThread = runThread; }
+    bool runThread() const { LockHolder locker(m_mutex); return m_runThread; }
+    void setRunThread(bool runThread) { LockHolder locker(m_mutex); m_runThread = runThread; }
 
     bool addToCurl(CURL* curlHandle);
     bool removeFromCurl(CURL* curlHandle);