Bug 537017 - fix crash in nsMsgFilterAfterTheFact::ApplyFilter() caused by async reset of 'm_curFolder'. r=jorgk
authorMagnus Melin <mkmelin+mozilla@iki.fi>
Fri, 28 Sep 2018 23:01:22 +0300
changeset 33260 4568dcf1dd5700335d893b7ce077eabf085f013e
parent 33259 cac40c2c829de36bcc009a375ba950753802dc90
child 33261 1dfd8879d65bea1103a01da0ef8edbd378925677
push id387
push userclokep@gmail.com
push dateMon, 10 Dec 2018 21:30:47 +0000
reviewersjorgk
bugs537017
Bug 537017 - fix crash in nsMsgFilterAfterTheFact::ApplyFilter() caused by async reset of 'm_curFolder'. r=jorgk 'm_curFolder' can be reset asynchronously by the copy service calling OnStopCopy(). So take a local copy here and use it throughout the function.
mailnews/base/search/src/nsMsgFilterService.cpp
--- a/mailnews/base/search/src/nsMsgFilterService.cpp
+++ b/mailnews/base/search/src/nsMsgFilterService.cpp
@@ -521,20 +521,25 @@ NS_IMETHODIMP nsMsgFilterAfterTheFact::O
 //   filters, and folders until done, unless it starts an async operation with
 //   a callback. The callback should call ApplyFilter again. It only returns
 //   an error if it is impossible to continue after attempting to continue the
 //   next filter action, filter, or folder.
 nsresult nsMsgFilterAfterTheFact::ApplyFilter()
 {
   nsresult rv;
   do { // error management block, break if unable to continue with filter.
+    // 'm_curFolder' can be reset asynchronously by the copy service
+    // calling OnStopCopy(). So take a local copy here and use it throughout the
+    // function.
+    nsCOMPtr<nsIMsgFolder> curFolder = m_curFolder;
     if (!m_curFilter)
       break; // Maybe not an error, we just need to call RunNextFilter();
-    if (!m_curFolder)
+    if (!curFolder)
       break; // Maybe not an error, we just need to call AdvanceToNextFolder();
+
     BREAK_IF_FALSE(m_searchHitHdrs, "No search headers object");
     // we're going to log the filter actions before firing them because some actions are async
     bool loggingEnabled = false;
     if (m_filters)
       (void)m_filters->GetLoggingEnabled(&loggingEnabled);
 
     nsCOMPtr<nsIArray> actionList;
     rv = m_curFilter->GetSortedActionList(getter_AddRefs(actionList));
@@ -581,36 +586,36 @@ nsresult nsMsgFilterAfterTheFact::ApplyF
       switch (actionType)
       {
       case nsMsgFilterAction::Delete:
         // we can't pass ourselves in as a copy service listener because the copy service
         // listener won't get called in several situations (e.g., the delete model is imap delete)
         // and we rely on the listener getting called to continue the filter application.
         // This means we're going to end up firing off the delete, and then subsequently
         // issuing a search for the next filter, which will block until the delete finishes.
-        m_curFolder->DeleteMessages(m_searchHitHdrs, m_msgWindow, false, false, nullptr, false /*allow Undo*/ );
+        curFolder->DeleteMessages(m_searchHitHdrs, m_msgWindow, false, false, nullptr, false /*allow Undo*/ );
 
         // don't allow any more filters on this message
         m_stopFiltering.AppendElements(m_searchHits);
         for (uint32_t i = 0; i < m_searchHits.Length(); i++)
-          m_curFolder->OrProcessingFlags(m_searchHits[i], nsMsgProcessingFlags::FilterToMove);
+          curFolder->OrProcessingFlags(m_searchHits[i], nsMsgProcessingFlags::FilterToMove);
         //if we are deleting then we couldn't care less about applying remaining filter actions
         m_nextAction = numActions;
         break;
 
       case nsMsgFilterAction::MoveToFolder:
         // Even if move fails we will not run additional actions, as they
         //   would not have run if move succeeded.
         m_nextAction = numActions;
         // Fall through to the copy case.
         MOZ_FALLTHROUGH;
       case nsMsgFilterAction::CopyToFolder:
       {
         nsCString uri;
-        m_curFolder->GetURI(uri);
+        curFolder->GetURI(uri);
         if (!actionTargetFolderUri.IsEmpty() &&
             !uri.Equals(actionTargetFolderUri))
         {
           nsCOMPtr<nsIRDFService> rdf = do_GetService("@mozilla.org/rdf/rdf-service;1",&rv);
           nsCOMPtr<nsIRDFResource> res;
           rv = rdf->GetResource(actionTargetFolderUri, getter_AddRefs(res));
           CONTINUE_IF_FAILURE(rv, "Could not get resource for action folder");
 
@@ -641,41 +646,41 @@ nsresult nsMsgFilterAfterTheFact::ApplyF
           }
           nsCOMPtr<nsIMsgCopyService> copyService = do_GetService(NS_MSGCOPYSERVICE_CONTRACTID, &rv);
           CONTINUE_IF_FAILURE(rv, "Could not get copy service")
 
           if (actionType == nsMsgFilterAction::MoveToFolder)
           {
             m_stopFiltering.AppendElements(m_searchHits);
             for (uint32_t i = 0; i < m_searchHits.Length(); i++)
-              m_curFolder->OrProcessingFlags(m_searchHits[i],
+              curFolder->OrProcessingFlags(m_searchHits[i],
                                              nsMsgProcessingFlags::FilterToMove);
           }
 
-          rv = copyService->CopyMessages(m_curFolder, m_searchHitHdrs,
+          rv = copyService->CopyMessages(curFolder, m_searchHitHdrs,
               destIFolder, actionType == nsMsgFilterAction::MoveToFolder,
               this, m_msgWindow, false);
           CONTINUE_IF_FAILURE(rv, "CopyMessages failed");
           return NS_OK; // OnStopCopy callback to continue;
         }
         else
           NS_WARNING("Move or copy failed, empty or unchanged destination");
       }
         break;
       case nsMsgFilterAction::MarkRead:
           // crud, no listener support here - we'll probably just need to go on and apply
           // the next filter, and, in the imap case, rely on multiple connection and url
           // queueing to stay out of trouble
-        m_curFolder->MarkMessagesRead(m_searchHitHdrs, true);
+        curFolder->MarkMessagesRead(m_searchHitHdrs, true);
         break;
       case nsMsgFilterAction::MarkUnread:
-        m_curFolder->MarkMessagesRead(m_searchHitHdrs, false);
+        curFolder->MarkMessagesRead(m_searchHitHdrs, false);
         break;
       case nsMsgFilterAction::MarkFlagged:
-        m_curFolder->MarkMessagesFlagged(m_searchHitHdrs, true);
+        curFolder->MarkMessagesFlagged(m_searchHitHdrs, true);
         break;
       case nsMsgFilterAction::KillThread:
       case nsMsgFilterAction::WatchThread:
         {
           for (uint32_t msgIndex = 0; msgIndex < m_searchHits.Length(); msgIndex++)
           {
             nsCOMPtr<nsIMsgDBHdr> msgHdr = do_QueryElementAt(m_searchHitHdrs, msgIndex);
             CONTINUE_IF_FALSE(msgHdr, "Could not get msg header");
@@ -713,39 +718,39 @@ nsresult nsMsgFilterAfterTheFact::ApplyF
             msgHdr->SetPriority(filterPriority);
           }
         }
         break;
       case nsMsgFilterAction::Label:
         {
           nsMsgLabelValue filterLabel;
           filterAction->GetLabel(&filterLabel);
-          m_curFolder->SetLabelForMessages(m_searchHitHdrs, filterLabel);
+          curFolder->SetLabelForMessages(m_searchHitHdrs, filterLabel);
         }
         break;
       case nsMsgFilterAction::AddTag:
         {
           nsCString keyword;
           filterAction->GetStrValue(keyword);
-          m_curFolder->AddKeywordsToMessages(m_searchHitHdrs, keyword);
+          curFolder->AddKeywordsToMessages(m_searchHitHdrs, keyword);
         }
         break;
       case nsMsgFilterAction::JunkScore:
         {
           nsAutoCString junkScoreStr;
           int32_t junkScore;
           filterAction->GetJunkScore(&junkScore);
           junkScoreStr.AppendInt(junkScore);
-          m_curFolder->SetJunkScoreForMessages(m_searchHitHdrs, junkScoreStr);
+          curFolder->SetJunkScoreForMessages(m_searchHitHdrs, junkScoreStr);
         }
         break;
       case nsMsgFilterAction::Forward:
         {
           nsCOMPtr<nsIMsgIncomingServer> server;
-          rv = m_curFolder->GetServer(getter_AddRefs(server));
+          rv = curFolder->GetServer(getter_AddRefs(server));
           CONTINUE_IF_FAILURE(rv, "Could not get server");
           nsCString forwardTo;
           filterAction->GetStrValue(forwardTo);
           CONTINUE_IF_FALSE(!forwardTo.IsEmpty(), "blank forwardTo URI");
           nsCOMPtr<nsIMsgComposeService> compService =
             do_GetService(NS_MSGCOMPOSESERVICE_CONTRACTID, &rv);
           CONTINUE_IF_FAILURE(rv, "Could not get compose service");
 
@@ -763,17 +768,17 @@ nsresult nsMsgFilterAfterTheFact::ApplyF
         break;
       case nsMsgFilterAction::Reply:
         {
           nsCString replyTemplateUri;
           filterAction->GetStrValue(replyTemplateUri);
           CONTINUE_IF_FALSE(!replyTemplateUri.IsEmpty(), "Empty reply template URI");
 
           nsCOMPtr<nsIMsgIncomingServer> server;
-          rv = m_curFolder->GetServer(getter_AddRefs(server));
+          rv = curFolder->GetServer(getter_AddRefs(server));
           CONTINUE_IF_FAILURE(rv, "Could not get server");
 
           nsCOMPtr<nsIMsgComposeService> compService = do_GetService(NS_MSGCOMPOSESERVICE_CONTRACTID, &rv);
           CONTINUE_IF_FAILURE(rv, "Could not get compose service");
           for (uint32_t msgIndex = 0; msgIndex < m_searchHits.Length(); msgIndex++)
           {
             nsCOMPtr<nsIMsgDBHdr> msgHdr = do_QueryElementAt(m_searchHitHdrs, msgIndex);
             CONTINUE_IF_FALSE(msgHdr, "Could not get msgHdr");
@@ -786,17 +791,17 @@ nsresult nsMsgFilterAfterTheFact::ApplyF
               }
             }
             CONTINUE_IF_FAILURE(rv, "ReplyWithTemplate failed");
           }
         }
         break;
       case nsMsgFilterAction::DeleteFromPop3Server:
         {
-          nsCOMPtr<nsIMsgLocalMailFolder> localFolder = do_QueryInterface(m_curFolder);
+          nsCOMPtr<nsIMsgLocalMailFolder> localFolder = do_QueryInterface(curFolder);
           CONTINUE_IF_FALSE(localFolder, "Current folder not a local folder");
           // This action ignores the deleteMailLeftOnServer preference
           rv = localFolder->MarkMsgsOnPop3Server(m_searchHitHdrs, POP3_FORCE_DEL);
           CONTINUE_IF_FAILURE(rv, "MarkMsgsOnPop3Server failed");
 
           nsCOMPtr<nsIMutableArray> partialMsgs;
           // Delete the partial headers. They're useless now
           //   that the server copy is being deleted.
@@ -808,47 +813,47 @@ nsresult nsMsgFilterAfterTheFact::ApplyF
             msgHdr->GetFlags(&flags);
             if (flags & nsMsgMessageFlags::Partial)
             {
               if (!partialMsgs)
                 partialMsgs = do_CreateInstance(NS_ARRAY_CONTRACTID, &rv);
               CONTINUE_IF_FALSE(partialMsgs, "Could not create partialMsgs array");
               partialMsgs->AppendElement(msgHdr);
               m_stopFiltering.AppendElement(m_searchHits[msgIndex]);
-              m_curFolder->OrProcessingFlags(m_searchHits[msgIndex],
+              curFolder->OrProcessingFlags(m_searchHits[msgIndex],
                                              nsMsgProcessingFlags::FilterToMove);
             }
           }
           if (partialMsgs)
           {
-            m_curFolder->DeleteMessages(partialMsgs, m_msgWindow, true, false, nullptr, false);
+            curFolder->DeleteMessages(partialMsgs, m_msgWindow, true, false, nullptr, false);
             CONTINUE_IF_FAILURE(rv, "Delete messages failed");
           }
         }
         break;
       case nsMsgFilterAction::FetchBodyFromPop3Server:
         {
-          nsCOMPtr<nsIMsgLocalMailFolder> localFolder = do_QueryInterface(m_curFolder);
+          nsCOMPtr<nsIMsgLocalMailFolder> localFolder = do_QueryInterface(curFolder);
           CONTINUE_IF_FALSE(localFolder, "current folder not local");
           nsCOMPtr<nsIMutableArray> messages(do_CreateInstance(NS_ARRAY_CONTRACTID, &rv));
           CONTINUE_IF_FAILURE(rv, "Could not create messages array");
           for (uint32_t msgIndex = 0; msgIndex < m_searchHits.Length(); msgIndex++)
           {
             nsCOMPtr<nsIMsgDBHdr> msgHdr = do_QueryElementAt(m_searchHitHdrs, msgIndex);
             CONTINUE_IF_FALSE(msgHdr, "Could not get msgHdr");
             uint32_t flags = 0;
             msgHdr->GetFlags(&flags);
             if (flags & nsMsgMessageFlags::Partial)
               messages->AppendElement(msgHdr);
           }
           uint32_t msgsToFetch;
           messages->GetLength(&msgsToFetch);
           if (msgsToFetch > 0)
           {
-            rv = m_curFolder->DownloadMessagesForOffline(messages, m_msgWindow);
+            rv = curFolder->DownloadMessagesForOffline(messages, m_msgWindow);
             CONTINUE_IF_FAILURE(rv, "DownloadMessagesForOffline failed");
           }
         }
         break;
 
       case nsMsgFilterAction::StopExecution:
         {
           // don't apply any more filters