bug 1242030: DONT-USE-REUSABLE Improve error handling in file/stream I/O. Part 2. r=jorgk. draft
authorISHIKAWA, Chiaki <ishikawa@yk.rim.or.jp>
Sat, 09 Nov 2019 10:59:42 +0900
changeset 81056 28ac72095d82940f9091ac285a031b2b1f2df00a
parent 81055 d7ce75a4e4bc7bc0470bed1fc552e7e581f39b68
child 81057 2f506aba41571a3d02a93e33160ef3d8cb4f4088
push id9744
push userishikawa@yk.rim.or.jp
push dateSat, 09 Nov 2019 02:01:06 +0000
treeherdertry-comm-central@56c83244ac0e [default view] [failures only]
reviewersjorgk
bugs1242030
bug 1242030: DONT-USE-REUSABLE Improve error handling in file/stream I/O. Part 2. r=jorgk. * * * temp.patch
mailnews/imap/src/nsImapIncomingServer.cpp
mailnews/imap/src/nsImapMailFolder.cpp
mailnews/imap/src/nsImapOfflineSync.cpp
mailnews/imap/src/nsImapProtocol.cpp
mailnews/imap/src/nsImapService.cpp
mailnews/imap/src/nsImapUndoTxn.cpp
--- a/mailnews/imap/src/nsImapIncomingServer.cpp
+++ b/mailnews/imap/src/nsImapIncomingServer.cpp
@@ -585,18 +585,20 @@ nsresult nsImapIncomingServer::DoomUrlIf
         mockChannel) {
       nsresult requestStatus;
       mockChannel->GetStatus(&requestStatus);
       if (NS_FAILED(requestStatus)) {
         nsresult res;
         *urlDoomed = true;
         nsImapProtocol::LogImapUrl("dooming url", aImapUrl);
 
-        mockChannel
-            ->Close();  // try closing it to get channel listener nulled out.
+        nsresult rv1 = mockChannel->Close();  // try closing it to get channel
+                                              // listener nulled out.
+
+        if (NS_FAILED(rv1)) NS_WARNING("mockChannel->Close() failed");
 
         if (aMailNewsUrl) {
           nsCOMPtr<nsICacheEntry> cacheEntry;
           res = aMailNewsUrl->GetMemCacheEntry(getter_AddRefs(cacheEntry));
           if (NS_SUCCEEDED(res) && cacheEntry) cacheEntry->AsyncDoom(nullptr);
           // we're aborting this url - tell listeners
           aMailNewsUrl->SetUrlState(false, NS_MSG_ERROR_URL_ABORTED);
         }
--- a/mailnews/imap/src/nsImapMailFolder.cpp
+++ b/mailnews/imap/src/nsImapMailFolder.cpp
@@ -915,19 +915,21 @@ NS_IMETHODIMP nsImapMailFolder::CreateCl
 
       // store the online name as the mailbox name in the db folder info
       // I don't think anyone uses the mailbox name, so we'll use it
       // to restore the online name when blowing away an imap db.
       if (folderInfo)
         folderInfo->SetMailboxName(NS_ConvertASCIItoUTF16(onlineName));
     }
 
+    // XXX TODO we should probably check the return value of the DB OPs?
     unusedDB->SetSummaryValid(true);
     unusedDB->Commit(nsMsgDBCommitType::kLargeCommit);
-    unusedDB->Close(true);
+    nsresult rv1 = unusedDB->Close(true);
+    if (NS_FAILED(rv1)) NS_WARNING("unusedDB->Close(true) failed");
     // don't want to hold onto this newly created db.
     child->SetMsgDatabase(nullptr);
   }
 
   if (!suppressNotification) {
     if (NS_SUCCEEDED(rv) && child) {
       NotifyItemAdded(child);
       child->NotifyFolderEvent(kFolderCreateCompleted);
@@ -3013,20 +3015,23 @@ NS_IMETHODIMP nsImapMailFolder::BeginCop
 
   if (!m_copyState->m_dataBuffer)
     m_copyState->m_dataBuffer = (char *)PR_CALLOC(COPY_BUFFER_SIZE + 1);
   NS_ENSURE_TRUE(m_copyState->m_dataBuffer, NS_ERROR_OUT_OF_MEMORY);
   m_copyState->m_dataBufferSize = COPY_BUFFER_SIZE;
   return NS_OK;
 }
 
+// Note: 'outputStream' is allocated in BeginCopy() as buffered stream into
+// m_copyState->m_msgFileStream and passed here from CopyData().
 NS_IMETHODIMP nsImapMailFolder::CopyDataToOutputStreamForAppend(
     nsIInputStream *aIStream, int32_t aLength, nsIOutputStream *outputStream) {
   uint32_t readCount;
   uint32_t writeCount;
+  uint32_t writeCount2;
   if (!m_copyState) m_copyState = new nsImapMailCopyState();
 
   if (aLength + m_copyState->m_leftOver > m_copyState->m_dataBufferSize) {
     char *newBuffer = (char *)PR_REALLOC(m_copyState->m_dataBuffer,
                                          aLength + m_copyState->m_leftOver + 1);
     NS_ENSURE_TRUE(newBuffer, NS_ERROR_OUT_OF_MEMORY);
     m_copyState->m_dataBuffer = newBuffer;
     m_copyState->m_dataBufferSize = aLength + m_copyState->m_leftOver;
@@ -3050,17 +3055,26 @@ NS_IMETHODIMP nsImapMailFolder::CopyData
   end = PL_strpbrk(start, "\r\n");
   if (end && *end == '\r' && *(end + 1) == '\n') linebreak_len = 2;
 
   while (start && end) {
     if (PL_strncasecmp(start, "X-Mozilla-Status:", 17) &&
         PL_strncasecmp(start, "X-Mozilla-Status2:", 18) &&
         PL_strncmp(start, "From - ", 7)) {
       rv = outputStream->Write(start, end - start, &writeCount);
-      rv = outputStream->Write(CRLF, 2, &writeCount);
+      // XXX TODO: Better error check/recovery based on rv, rv2 and writeCount,
+      // writeCount2 for errors in writing to almost full file system,
+      // network file system with transient error, etc. Maybe errno
+      // as well to retry I/O for EINTR type of issues.
+      rv = outputStream->Write(start, end - start, &writeCount);
+      MOZ_ASSERT(NS_SUCCEEDED(rv) && (int)writeCount == (end - start),
+                 "outputStream->Write(start, ...) failed");
+      nsresult rv2 = outputStream->Write(CRLF, 2, &writeCount2);
+      if (!(NS_SUCCEEDED(rv2) && writeCount2 == 2))
+        NS_WARNING("outputStream->Write(CRLF, ...) failed");
     }
     start = end + linebreak_len;
     if (start >= m_copyState->m_dataBuffer + m_copyState->m_leftOver) {
       m_copyState->m_leftOver = 0;
       break;
     }
     linebreak_len = 1;
 
@@ -3102,17 +3116,20 @@ NS_IMETHODIMP nsImapMailFolder::CopyData
   }
   return rv;
 }
 
 NS_IMETHODIMP nsImapMailFolder::EndCopy(bool copySucceeded) {
   nsresult rv = copySucceeded ? NS_OK : NS_ERROR_FAILURE;
   if (copySucceeded && m_copyState && m_copyState->m_msgFileStream) {
     nsCOMPtr<nsIUrlListener> urlListener;
-    m_copyState->m_msgFileStream->Close();
+    nsresult rv1 = m_copyState->m_msgFileStream->Close();
+    if (NS_FAILED(rv1))
+      NS_WARNING("m_copyState->m_msgFileStream->Close() failed");
+    m_copyState->m_msgFileStream = nullptr;
     // m_tmpFile can be stale because we wrote to it
     nsCOMPtr<nsIFile> tmpFile;
     m_copyState->m_tmpFile->Clone(getter_AddRefs(tmpFile));
     m_copyState->m_tmpFile = tmpFile;
     nsCOMPtr<nsIImapService> imapService =
         do_GetService(NS_IMAPSERVICE_CONTRACTID, &rv);
     NS_ENSURE_SUCCESS(rv, rv);
 
@@ -4168,29 +4185,38 @@ nsImapMailFolder::SetupMsgWriteStream(ns
   rv = MsgNewBufferedFileOutputStream(
       getter_AddRefs(m_tempMessageStream), aFile,
       PR_WRONLY | PR_CREATE_FILE | PR_TRUNCATE, 00700);
   if (m_tempMessageStream && addDummyEnvelope) {
     nsAutoCString result;
     char *ct;
     uint32_t writeCount;
     time_t now = time((time_t *)0);
+    // ctime() returns |Www Mmm dd hh:mm:ss yyyy| followed by \n.
     ct = ctime(&now);
-    ct[24] = 0;
+    ct[24] = 0;  // Kill \n.
     result = "From - ";
     result += ct;
     result += MSG_LINEBREAK;
-
-    m_tempMessageStream->Write(result.get(), result.Length(), &writeCount);
     result = "X-Mozilla-Status: 0001";
     result += MSG_LINEBREAK;
-    m_tempMessageStream->Write(result.get(), result.Length(), &writeCount);
     result = "X-Mozilla-Status2: 00000000";
     result += MSG_LINEBREAK;
-    m_tempMessageStream->Write(result.get(), result.Length(), &writeCount);
+    nsresult rv2 =
+        m_tempMessageStream->Write(result.get(), result.Length(), &writeCount);
+    // MOZ_ASSERT does not get compiled into non-debug binary. We need the check
+    // for released binary to detect and report I/O error situation. Besides,
+    // the nsresult variable NEEDs to be used after assignment to avoid compile
+    // error. The next statement needs to do some action even in non-debug build
+    // so that rv2 is used effectively. Otherwise the compiler complains and
+    // build fails.
+    if (!(NS_SUCCEEDED(rv2) && writeCount == result.Length()))
+      NS_WARNING("m_tempMessageStream->Write(result.get(), ...) failed");
+    // XXX TODO we should return error based on rv2 here. Whether we should
+    // close m_tempMessageStream here is arguable.
   }
   return rv;
 }
 
 NS_IMETHODIMP nsImapMailFolder::DownloadMessagesForOffline(
     nsIArray *messages, nsIMsgWindow *window) {
   nsAutoCString messageIds;
   nsTArray<nsMsgKey> srcKeyArray;
@@ -4276,24 +4302,30 @@ nsImapMailFolder::ParseAdoptedMsgLine(co
   } while (nextLine && *nextLine);
 
   if (m_tempMessageStream) {
     nsCOMPtr<nsISeekableStream> seekable(
         do_QueryInterface(m_tempMessageStream));
     if (seekable) seekable->Seek(PR_SEEK_END, 0);
     rv = m_tempMessageStream->Write(adoptedMessageLine,
                                     PL_strlen(adoptedMessageLine), &count);
-    NS_ENSURE_SUCCESS(rv, rv);
+    MOZ_ASSERT(NS_SUCCEEDED(rv) && PL_strlen(adoptedMessageLine) == count,
+               "m_tempMessageStream->Write(adoptedMessageLine, ...) failed");
+    // Return error if Write() didn't write correct number of bytes.
+    if (NS_SUCCEEDED(rv) && count != PL_strlen(adoptedMessageLine))
+      return NS_ERROR_FAILURE;
+    if (NS_FAILED(rv)) return rv;
   }
   return NS_OK;
 }
 
 void nsImapMailFolder::EndOfflineDownload() {
   if (m_tempMessageStream) {
-    m_tempMessageStream->Close();
+    nsresult rv = m_tempMessageStream->Close();
+    if (NS_FAILED(rv)) NS_WARNING("m_tempMessageStream->Close() failed");
     m_tempMessageStream = nullptr;
     ReleaseSemaphore(static_cast<nsIMsgFolder *>(this));
     if (mDatabase) mDatabase->Commit(nsMsgDBCommitType::kLargeCommit);
   }
   m_offlineHeader = nullptr;
 }
 
 NS_IMETHODIMP
@@ -4413,19 +4445,24 @@ nsImapMailFolder::OnlineCopyCompleted(ns
     srcFolder = do_QueryInterface(m_copyState->m_srcSupport, &rv);
     if (srcFolder) srcFolder->NotifyFolderEvent(kDeleteOrMoveMsgCompleted);
   } else
     rv = NS_ERROR_FAILURE;
 
   return rv;
 }
 
+// We probably should return failure code if Close() failed.
 NS_IMETHODIMP
 nsImapMailFolder::CloseMockChannel(nsIImapMockChannel *aChannel) {
-  aChannel->Close();
+  nsresult rv = aChannel->Close();
+  if (NS_FAILED(rv)) {
+    NS_WARNING("aChannel->Close() failed");
+    return rv;
+  }
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsImapMailFolder::ReleaseUrlCacheEntry(nsIMsgMailNewsUrl *aUrl) {
   NS_ENSURE_ARG_POINTER(aUrl);
   return aUrl->SetMemCacheEntry(nullptr);
 }
@@ -5666,17 +5703,18 @@ NS_IMETHODIMP nsImapMailFolder::SetAclFl
     if (mDatabase) {
       rv = mDatabase->GetDBFolderInfo(getter_AddRefs(dbFolderInfo));
       if (NS_SUCCEEDED(rv) && dbFolderInfo)
         dbFolderInfo->SetUint32Property("aclFlags", aclFlags);
       // if setting the acl flags caused us to open the db, release the ref
       // because on startup, we might get acl on all folders,which will
       // leave a lot of db's open.
       if (!dbWasOpen) {
-        mDatabase->Close(true /* commit changes */);
+        nsresult rv1 = mDatabase->Close(true);  // Commit changes.
+        if (NS_FAILED(rv1)) NS_WARNING("mDatabase->Close(true) failed");
         mDatabase = nullptr;
       }
     }
   }
   return rv;
 }
 
 NS_IMETHODIMP nsImapMailFolder::GetAclFlags(uint32_t *aclFlags) {
@@ -5694,17 +5732,18 @@ NS_IMETHODIMP nsImapMailFolder::GetAclFl
       if (NS_SUCCEEDED(rv) && dbFolderInfo) {
         rv = dbFolderInfo->GetUint32Property("aclFlags", 0, aclFlags);
         m_aclFlags = *aclFlags;
       }
       // if getting the acl flags caused us to open the db, release the ref
       // because on startup, we might get acl on all folders,which will
       // leave a lot of db's open.
       if (!dbWasOpen) {
-        mDatabase->Close(true /* commit changes */);
+        nsresult rv1 = mDatabase->Close(true);  // Commit changes.
+        if (NS_FAILED(rv1)) NS_WARNING("mDatabase->Close(true) failed");
         mDatabase = nullptr;
       }
     }
   } else
     *aclFlags = m_aclFlags;
   return NS_OK;
 }
 
@@ -6443,25 +6482,29 @@ nsresult nsImapMailFolder::CopyOfflineMs
         rv = inputStream->Read(inputBuffer, FILE_IO_BUFFER_SIZE, &bytesRead);
         if (NS_SUCCEEDED(rv) && bytesRead > 0) {
           rv = outputStream->Write(inputBuffer,
                                    std::min((int32_t)bytesRead, bytesLeft),
                                    &bytesWritten);
           NS_ASSERTION(
               (int32_t)bytesWritten == std::min((int32_t)bytesRead, bytesLeft),
               "wrote out incorrect number of bytes");
-        } else
+        } else {
+          // XXX TODO Check for network errors, etc. Would be handled in
+          // short-read patch.
           break;
+        }
         bytesLeft -= bytesRead;
       }
       PR_FREEIF(inputBuffer);
     }
   }
   if (NS_SUCCEEDED(rv)) {
-    outputStream->Flush();
+    nsresult rv2 = outputStream->Flush();
+    if (NS_FAILED(rv2)) NS_WARNING("outputStream->Flush() failed");
     uint32_t resultFlags;
     destHdr->OrFlags(nsMsgMessageFlags::Offline, &resultFlags);
     destHdr->SetOfflineMessageSize(messageSize);
   }
   return rv;
 }
 
 nsresult nsImapMailFolder::FindOpenRange(nsMsgKey &fakeBase,
@@ -6549,17 +6592,17 @@ nsresult nsImapMailFolder::CopyMessagesO
       // N.B. We must not return out of the for loop - we need the matching
       // end notifications to be sent.
       // We don't need to acquire the semaphor since this is synchronous
       // on the UI thread but we should check if the offline store is locked.
       bool isLocked;
       GetLocked(&isLocked);
       nsCOMPtr<nsIInputStream> inputStream;
       bool reusable = false;
-      nsCOMPtr<nsIOutputStream> outputStream;
+      nsCOMPtr<nsIOutputStream> outputStream = nullptr;
       nsTArray<nsMsgKey> addedKeys;
       nsTArray<nsMsgKey> srcKeyArray;
       nsCOMArray<nsIMsgDBHdr> addedHdrs;
       nsCOMArray<nsIMsgDBHdr> srcMsgs;
       nsOfflineImapOperationType moveCopyOpType;
       nsOfflineImapOperationType deleteOpType =
           nsIMsgOfflineImapOperation::kDeletedMsg;
       if (!deleteToTrash)
@@ -6670,24 +6713,43 @@ nsresult nsImapMailFolder::CopyMessagesO
             newMailHdr->SetUint32Property("pseudoHdr", 1);
             if (!reusable)
               (void)srcFolder->GetMsgInputStream(newMailHdr, &reusable,
                                                  getter_AddRefs(inputStream));
 
             if (inputStream && hasMsgOffline && !isLocked) {
               rv = GetOfflineStoreOutputStream(newMailHdr,
                                                getter_AddRefs(outputStream));
+              // XXX TODO  WARNING. With the NS_ENSURE_SUCCESS() below,
+              // we are breaking out of the loop despite earlier comment about
+              // "N.B. We must not return out of the for loop - we need the
+              // matching end notifications to be sent." So we may want to open
+              // code the line below to print error/warning message AND continue
+              // the loop.
               NS_ENSURE_SUCCESS(rv, rv);
 
               CopyOfflineMsgBody(srcFolder, newMailHdr, mailHdr, inputStream,
                                  outputStream);
               nsCOMPtr<nsIMsgPluggableStore> offlineStore;
               (void)GetMsgStore(getter_AddRefs(offlineStore));
-              if (offlineStore)
-                offlineStore->FinishNewMessage(outputStream, newMailHdr);
+              if (offlineStore) {
+                // bool closed = false;
+                nsresult rv2 =
+                    offlineStore->FinishNewMessage(outputStream, newMailHdr);
+                if (NS_FAILED(rv2))
+                  NS_WARNING("msgStore->FinishNewMessage(...) failed");
+
+                // This runs in a loop and |outputStream| is always recreated
+                // by |GetOfflineStoreOutputStream()| call above when it is
+                // necesary to access it. nsresult rv3 = NS_OK;
+                outputStream = nullptr;
+
+                // if (!(NS_SUCCEEDED(rv3)))
+                //  NS_WARNING("outputStream->Close() failed");
+              }
             } else
               database->MarkOffline(fakeBase + sourceKeyIndex, false, nullptr);
 
             nsCOMPtr<nsIMsgOfflineImapOperation> destOp;
             database->GetOfflineOpForKey(fakeBase + sourceKeyIndex, true,
                                          getter_AddRefs(destOp));
             if (destOp) {
               // check if this is a move back to the original mailbox, in which
@@ -6713,17 +6775,17 @@ nsresult nsImapMailFolder::CopyMessagesO
             else
               sourceMailDB->MarkImapDeleted(msgKey, true,
                                             nullptr);  // offline delete
           }
           if (successfulCopy)
             // This is for both moves and copies
             msgHdrsCopied->AppendElement(mailHdr);
         }
-      }
+      }  // for-loop
       EnableNotifications(nsIMsgFolder::allMessageCountNotifications, true);
       RefPtr<nsImapOfflineTxn> addHdrMsgTxn = new nsImapOfflineTxn(
           this, &addedKeys, nullptr, this, isMove,
           nsIMsgOfflineImapOperation::kAddedHeader, addedHdrs);
       if (addHdrMsgTxn && txnMgr) txnMgr->DoTransaction(addHdrMsgTxn);
       RefPtr<nsImapOfflineTxn> undoMsgTxn =
           new nsImapOfflineTxn(srcFolder, &srcKeyArray, messageIds.get(), this,
                                isMove, moveCopyOpType, srcMsgs);
@@ -6753,17 +6815,24 @@ nsresult nsImapMailFolder::CopyMessagesO
           if (mFlags & nsMsgFolderFlags::Trash)
             undoMsgTxn->SetTransactionType(nsIMessenger::eDeleteMsg);
           else
             undoMsgTxn->SetTransactionType(nsIMessenger::eMoveMsg);
         } else
           undoMsgTxn->SetTransactionType(nsIMessenger::eCopyMsg);
         if (txnMgr) txnMgr->DoTransaction(undoMsgTxn);
       }
-      if (outputStream) outputStream->Close();
+      if (outputStream) {
+        nsresult rv3 = outputStream->Close();
+        // The following statement needs to perform an action with side-effect
+        // Or reference to |rv3| is optimized out in non-debug build,
+        // and compiler complains and build fails on tryserver
+        if (NS_FAILED(rv3)) NS_WARNING("outputStream->Close() failed");
+        outputStream = nullptr;
+      }
 
       if (isMove) sourceMailDB->Commit(nsMsgDBCommitType::kLargeCommit);
       database->Commit(nsMsgDBCommitType::kLargeCommit);
       SummaryChanged();
       srcFolder->SummaryChanged();
     }
     if (txnMgr) txnMgr->EndBatch(false);
   }
@@ -6947,17 +7016,17 @@ nsImapMailFolder::CopyMessages(
   // in theory, if allowUndo is true, then this is a user initiated
   // action, and we should do it pseudo-offline. If it's not
   // user initiated (e.g., mail filters firing), then allowUndo is
   // false, and we should just do the action.
   if (!WeAreOffline() && sameServer && allowUndo) {
     // complete the copy operation as in offline mode
     rv = CopyMessagesOffline(srcFolder, messages, isMove, msgWindow, listener);
 
-    NS_WARNING_ASSERTION(NS_SUCCEEDED(rv), "error offline copy");
+    if (NS_FAILED(rv)) NS_WARNING("error offline copy");
     // We'll warn if this fails, but we should still try to play back
     // offline ops, because it's possible the copy got far enough to
     // create the offline ops.
 
     // We make sure that the source folder is an imap folder by limiting
     // pseudo-offline operations to the same imap server. If we extend the code
     // to cover non imap folders in the future (i.e. imap folder->local folder),
     // then the following downcast will cause either a crash or compiler error.
@@ -7684,27 +7753,43 @@ nsresult nsImapMailFolder::CopyFileToOff
     rv = NS_OK;
     msgParser->SetState(nsIMsgParseMailMsgState::ParseHeadersState);
     msgParser->SetNewMsgHdr(fakeHdr);
     bool needMoreData = false;
     char *newLine = nullptr;
     uint32_t numBytesInLine = 0;
     if (offlineStore) {
       const char *envelope = "From " CRLF;
-      offlineStore->Write(envelope, strlen(envelope), &bytesWritten);
+      nsresult rv2 =
+          offlineStore->Write(envelope, strlen(envelope), &bytesWritten);
+      // XXX TODO Better error reporting,
+      // maybe we need do return error code at the end of the loop?
+      // MOZ_ASSERT does not get compiled into non-debug binary. We need the
+      // check for released binary to detect and report I/O error situation.
+      // Besides, the nsresult variable NEEDs to be used after assignment to
+      // avoid compile error.
+      if (!(NS_SUCCEEDED(rv2) && bytesWritten == strlen(envelope)))
+        NS_WARNING("offlineStore->Write(envelope, ...) failed");
       fileSize += bytesWritten;
-    }
+    } else {
+      NS_WARNING("offlineStore is nullptr");
+    }
+
     do {
       newLine = inputStreamBuffer->ReadNextLine(inputStream, numBytesInLine,
                                                 needMoreData);
       if (newLine) {
         msgParser->ParseAFolderLine(newLine, numBytesInLine);
-        if (offlineStore)
+        if (offlineStore) {
           rv = offlineStore->Write(newLine, numBytesInLine, &bytesWritten);
-
+          // XXX TODO Better network error handling, etc.
+          MOZ_ASSERT(NS_SUCCEEDED(rv) && bytesWritten == numBytesInLine,
+                     "offlineStore->Write(newLine, numBytesInLine, "
+                     "&bytesWritten) failed");
+        }
         free(newLine);
         NS_ENSURE_SUCCESS(rv, rv);
       }
     } while (newLine);
 
     msgParser->FinishHeader();
     uint32_t resultFlags;
     if (offlineStore)
@@ -7716,35 +7801,49 @@ nsresult nsImapMailFolder::CopyFileToOff
     mDatabase->AddNewHdrToDB(fakeHdr, true /* notify */);
 
     // Call FinishNewMessage before setting pending attributes, as in
     //   maildir it copies from tmp to cur and may change the storeToken
     //   to get a unique filename.
     if (offlineStore) {
       nsCOMPtr<nsIMsgPluggableStore> msgStore;
       GetMsgStore(getter_AddRefs(msgStore));
-      if (msgStore) msgStore->FinishNewMessage(offlineStore, fakeHdr);
+      if (msgStore) {
+        nsresult rv2 = msgStore->FinishNewMessage(offlineStore, fakeHdr);
+        if (NS_FAILED(rv2))
+          NS_WARNING("msgStore->FinishNewMessage(...) failed");
+        // XXX should the next libe be msgStore = nullptr;
+        offlineStore = nullptr;
+      }
     }
 
     nsCOMPtr<nsIMutableArray> messages(
         do_CreateInstance(NS_ARRAY_CONTRACTID, &rv));
     NS_ENSURE_SUCCESS(rv, rv);
     messages->AppendElement(fakeHdr);
 
     // We are copying from a file to offline store so set offline flag.
     SetPendingAttributes(messages, false, true);
 
     // Gloda needs this notification to index the fake message.
     nsCOMPtr<nsIMsgFolderNotificationService> notifier(
         do_GetService(NS_MSGNOTIFICATIONSERVICE_CONTRACTID));
     if (notifier) notifier->NotifyMsgsClassified(messages, false, false);
-    inputStream->Close();
+    nsresult rv3 = inputStream->Close();
+    if (NS_FAILED(rv3)) NS_WARNING("inputStream->Close() failed");
     inputStream = nullptr;
   }
-  if (offlineStore) offlineStore->Close();
+  if (offlineStore) {
+    nsresult rv3 = offlineStore->Close();
+    if (NS_FAILED(rv3)) NS_WARNING("offlineStore->Close() failed");
+    offlineStore = nullptr;
+  }
+  // XXX The original intent seemed to be to return the error
+  // of Write failure only, but we need to take care of Flush/Close errors
+  // as well as the failure of FinishNewMessage().
   return rv;
 }
 
 nsresult nsImapMailFolder::OnCopyCompleted(nsISupports *srcSupport,
                                            nsresult rv) {
   // if it's a file, and the copy succeeded, then fcc the offline
   // store, and add a kMoveResult offline op.
   if (NS_SUCCEEDED(rv) && m_copyState) {
@@ -8077,19 +8176,22 @@ NS_IMETHODIMP nsImapMailFolder::RenameCl
         CopyASCIItoUTF16(onlineName, unicodeOnlineName);
         folderInfo->SetMailboxName(unicodeOnlineName);
       }
       bool changed = false;
       msgFolder->MatchOrChangeFilterDestination(
           child, false /*caseInsensitive*/, &changed);
       if (changed) msgFolder->AlertFilterChanged(msgWindow);
     }
+    // XXX TODO We should check the error of DB OPs?
     unusedDB->SetSummaryValid(true);
     unusedDB->Commit(nsMsgDBCommitType::kLargeCommit);
-    unusedDB->Close(true);
+    nsresult rv2 = unusedDB->Close(true);
+    if (NS_FAILED(rv2)) NS_WARNING("unusedDB->Close(true) failed");
+    unusedDB = nullptr;
     child->RenameSubFolders(msgWindow, msgFolder);
     nsCOMPtr<nsIMsgFolder> msgParent;
     msgFolder->GetParent(getter_AddRefs(msgParent));
     msgFolder->SetParent(nullptr);
     // Reset online status now that the folder is renamed.
     nsCOMPtr<nsIMsgImapMailFolder> oldImapFolder = do_QueryInterface(msgFolder);
     if (oldImapFolder) oldImapFolder->SetVerifiedAsOnlineFolder(false);
     nsCOMPtr<nsIMsgFolderNotificationService> notifier(
--- a/mailnews/imap/src/nsImapOfflineSync.cpp
+++ b/mailnews/imap/src/nsImapOfflineSync.cpp
@@ -400,30 +400,38 @@ void nsImapOfflineSync::ProcessAppendMsg
       if (NS_WARN_IF(NS_FAILED(rv))) break;
       MOZ_ASSERT(bytesWritten == bytesRead,
                  "wrote out incorrect number of bytes");
       bytesLeft -= bytesRead;
     }
     PR_FREEIF(inputBuffer);
 
     // rv could have an error from Read/Write.
+    if (NS_FAILED(rv))
+      NS_WARNING(
+          "offlineStoreInputStream->Read() or outputStream->Write() failed");
+
     nsresult rv2 = outputStream->Close();
     if (NS_FAILED(rv2)) {
-      NS_WARNING("ouputStream->Close() failed");
+      NS_WARNING("outputStream->Close() failed");
     }
     outputStream = nullptr;  // Don't try to close it again below.
 
     // rv: Read/Write, rv2: Close
     if (NS_FAILED(rv) || NS_FAILED(rv2)) {
       // This Remove() will fail under Windows if the output stream
       // fails to close above.
       mozilla::Unused << NS_WARN_IF(NS_FAILED(tmpFile->Remove(false)));
       break;
     }
 
+#ifdef DEBUG
+    NS_WARNING("(debug) we are cloning the temporary file");
+#endif
+
     nsCOMPtr<nsIFile> cloneTmpFile;
     // clone the tmp file to defeat nsIFile's stat/size caching.
     tmpFile->Clone(getter_AddRefs(cloneTmpFile));
     m_curTempFile = cloneTmpFile;
     nsCOMPtr<nsIMsgCopyService> copyService =
         do_GetService(NS_MSGCOPYSERVICE_CONTRACTID);
 
     // CopyFileMessage returns error async to this->OnStopCopy
--- a/mailnews/imap/src/nsImapProtocol.cpp
+++ b/mailnews/imap/src/nsImapProtocol.cpp
@@ -990,21 +990,25 @@ void nsImapProtocol::ReleaseUrlState(boo
     MutexAutoLock mon(mLock);
     if (m_transport) {
       m_transport->SetSecurityCallbacks(nullptr);
       m_transport->SetEventSink(nullptr, nullptr);
     }
   }
 
   if (m_mockChannel && !rerunning) {
+    // XXX TODO We should perform I/O error check/recover for CloseMockChannel.
     // Proxy the close of the channel to the ui thread.
     if (m_imapMailFolderSink)
       m_imapMailFolderSink->CloseMockChannel(m_mockChannel);
-    else
-      m_mockChannel->Close();
+    else {
+      nsresult rv3 = m_mockChannel->Close();
+      if (NS_FAILED(rv3)) NS_WARNING("m_mockChannel->Close() failed");
+      m_mockChannel = nullptr;
+    }
 
     {
       // grab a lock so m_mockChannel doesn't get cleared out
       // from under us.
       MutexAutoLock mon(mLock);
       if (m_mockChannel) {
         // Proxy the release of the channel to the main thread.  This is
         // something that the xpcom proxy system should do for us!
@@ -1108,17 +1112,21 @@ NS_IMETHODIMP nsImapProtocol::Run() {
   }
 
   if (m_runningUrl) {
     NS_ReleaseOnMainThreadSystemGroup("nsImapProtocol::m_runningUrl",
                                       m_runningUrl.forget());
   }
 
   // close streams via UI thread if it's not already done
-  if (m_imapProtocolSink) m_imapProtocolSink->CloseStreams();
+  if (m_imapProtocolSink) {
+    nsresult rv3 = m_imapProtocolSink->CloseStreams();
+    if (NS_FAILED(rv3)) NS_WARNING("m_imapProtocolSink->CloseStreams() failed");
+    m_imapProtocolSink = nullptr;
+  }
 
   m_imapMailFolderSink = nullptr;
   m_imapMessageSink = nullptr;
 
   // shutdown this thread, but do it from the main thread
   nsCOMPtr<nsIRunnable> ev = new nsImapThreadShutdownEvent(m_iThread);
   if (NS_FAILED(NS_DispatchToMainThread(ev)))
     NS_WARNING("Failed to dispatch nsImapThreadShutdownEvent");
@@ -1140,24 +1148,26 @@ NS_IMETHODIMP nsImapProtocol::CloseStrea
   MOZ_ASSERT(NS_IsMainThread(),
              "CloseStreams() should not be called from an off UI thread");
 
   {
     MutexAutoLock mon(mLock);
     if (m_transport) {
       // make sure the transport closes (even if someone is still indirectly
       // referencing it).
-      m_transport->Close(NS_ERROR_ABORT);
+      nsresult rv3 = m_transport->Close(NS_ERROR_ABORT);
+      if (NS_FAILED(rv3)) NS_WARNING("m_transport->Close() failed");
       m_transport = nullptr;
     }
     m_inputStream = nullptr;
     m_outputStream = nullptr;
     m_channelListener = nullptr;
     if (m_mockChannel) {
-      m_mockChannel->Close();
+      nsresult rv3 = m_mockChannel->Close();
+      if (NS_FAILED(rv3)) NS_WARNING("m_mockChannel->Close() failed");
       m_mockChannel = nullptr;
     }
     m_channelInputStream = nullptr;
     m_channelOutputStream = nullptr;
 
     // Close scope because we must let go of the monitor before calling
     // RemoveConnection to unblock anyone who tries to get a monitor to the
     // protocol object while holding onto a monitor to the server.
@@ -1290,16 +1300,17 @@ void nsImapProtocol::TellThreadToDie() {
 
     if (NS_SUCCEEDED(rv) && isAlive && TestFlag(IMAP_CONNECTION_IS_OPEN) &&
         NS_SUCCEEDED(GetConnectionStatus()) && m_outputStream)
       Logout(true, connectionIdle);
   }
   PR_CExitMonitor(this);
   // close streams via UI thread
   if (m_imapProtocolSink) {
+    // XXX TODO Probably we should perform error check/recovery.
     m_imapProtocolSink->CloseStreams();
     m_imapProtocolSink = nullptr;
   }
   Log("TellThreadToDie", nullptr, "close socket connection");
 
   {
     ReentrantMonitorAutoEnter mon(m_threadDeathMonitor);
     m_threadShouldDie = true;
@@ -1703,23 +1714,36 @@ bool nsImapProtocol::ProcessCurrentURL()
             }
           }
           if (NS_FAILED(rv)) {
             nsAutoCString logLine("STARTTLS negotiation failed. Error 0x");
             logLine.AppendInt(static_cast<uint32_t>(rv), 16);
             Log("ProcessCurrentURL", nullptr, logLine.get());
             if (m_socketType == nsMsgSocketType::alwaysSTARTTLS) {
               SetConnectionStatus(rv);  // stop netlib
-              m_transport->Close(rv);
+
+              if (m_transport) {
+                nsresult rv3 = m_transport->Close(rv);
+                if (NS_FAILED(rv3)) NS_WARNING("m_transport->Close(rv) failed");
+                m_transport = nullptr;
+              } else {
+                NS_WARNING("m_transport was null.");
+              }
             } else if (m_socketType == nsMsgSocketType::trySTARTTLS)
               m_imapServerSink->UpdateTrySTARTTLSPref(false);
           }
         } else if (m_socketType == nsMsgSocketType::alwaysSTARTTLS) {
           SetConnectionStatus(NS_ERROR_FAILURE);  // stop netlib
-          if (m_transport) m_transport->Close(rv);
+          if (m_transport) {
+            nsresult rv3 = m_transport->Close(rv);
+            if (NS_FAILED(rv3)) NS_WARNING("m_transport->Close() failed");
+            m_transport = nullptr;
+          } else {
+            NS_WARNING("m_transport was null.");
+          }
         } else if (m_socketType == nsMsgSocketType::trySTARTTLS) {
           // STARTTLS failed, so downgrade socket type
           m_imapServerSink->UpdateTrySTARTTLSPref(false);
         }
       } else if (m_socketType == nsMsgSocketType::trySTARTTLS) {
         // we didn't know the server supported TLS when we created
         // the socket, so we're going to retry with a STARTTLS socket
         if (GetServerStateParser().GetCapabilityFlag() &
@@ -1955,18 +1979,25 @@ nsresult nsImapProtocol::SendData(const 
           "Logging suppressed for this command (it probably contained "
           "authentication information)");
 
     {
       // don't allow someone to close the stream/transport out from under us
       // this can happen when the ui thread calls TellThreadToDie.
       PR_CEnterMonitor(this);
       uint32_t n;
-      if (m_outputStream)
+      if (m_outputStream) {
+        // XXX TODO handle network errors, etc.
         rv = m_outputStream->Write(dataBuffer, PL_strlen(dataBuffer), &n);
+        if (NS_FAILED(rv) || PL_strlen(dataBuffer) != n) {
+          NS_WARNING(
+              "m_outputStream->Write(dataBuffer, PL_strlen(dataBuffer), &n) "
+              "failed.");
+        }
+      }
       PR_CExitMonitor(this);
     }
     if (NS_FAILED(rv)) {
       Log("SendData", nullptr, "clearing IMAP_CONNECTION_IS_OPEN");
       // the connection died unexpectedly! so clear the open connection flag
       ClearFlag(IMAP_CONNECTION_IS_OPEN);
       TellThreadToDie();
       SetConnectionStatus(rv);
@@ -3700,16 +3731,23 @@ void nsImapProtocol::PostLineDownLoadEve
         nsresult rv = m_channelOutputStream->Write(line, byteCount, &count);
         NS_ASSERTION(count == byteCount,
                      "IMAP channel pipe couldn't buffer entire write");
         if (NS_SUCCEEDED(rv)) {
           m_channelListener->OnDataAvailable(m_mockChannel,
                                              m_channelInputStream, 0, count);
         }
         // else some sort of explosion?
+        // XXX TODO We should perform error recovery. We only print the error
+        // for now.
+        else {
+          MOZ_ASSERT(NS_SUCCEEDED(rv),
+                     "m_channelOutputStream->Write(line, byteCount, &count) "
+                     "returned an error;");
+        }
       }
     }
     if (m_runningUrl)
       m_runningUrl->GetStoreResultsOffline(&echoLineToMessageSink);
 
     m_bytesToChannel += byteCount;
     if (m_imapMessageSink && line && echoLineToMessageSink &&
         !GetPseudoInterrupted())
@@ -5959,17 +5997,17 @@ void nsImapProtocol::UploadMessageFromFi
       ParseIMAPandCheckForNewMail();
       if (!GetServerStateParser().LastCommandSuccessful()) goto done;
     }
 
     totalSize = fileSize;
     readCount = 0;
     while (NS_SUCCEEDED(rv) && !eof && totalSize > 0) {
       rv = fileInputStream->Read(dataBuffer, COPY_BUFFER_SIZE, &readCount);
-      if (NS_SUCCEEDED(rv) && !readCount) rv = NS_ERROR_FAILURE;
+      if (NS_SUCCEEDED(rv) && readCount == 0) rv = NS_ERROR_FAILURE;
 
       if (NS_SUCCEEDED(rv)) {
         NS_ASSERTION(readCount <= (uint32_t)totalSize,
                      "got more bytes than there should be");
         dataBuffer[readCount] = 0;
         rv = SendData(dataBuffer);
         totalSize -= readCount;
         PercentProgressUpdateEvent(nullptr, fileSize - totalSize, fileSize);
@@ -6045,17 +6083,23 @@ void nsImapProtocol::UploadMessageFromFi
             }
           }
         }
       }
     }
   }
 done:
   PR_Free(dataBuffer);
-  if (fileInputStream) fileInputStream->Close();
+  if (fileInputStream) {
+    nsresult rv3 = fileInputStream->Close();
+    if (NS_FAILED(rv3)) NS_WARNING("fileInputStream->Close() failed;");
+    fileInputStream = nullptr;  // code uniformity
+  } else {
+    NS_WARNING("fileInputStream was null");
+  }
 }
 
 // caller must free using PR_Free
 char *nsImapProtocol::OnCreateServerSourceFolderPathString() {
   char *sourceMailbox = nullptr;
   char hierarchyDelimiter = 0;
   char onlineDelimiter = 0;
   m_runningUrl->GetOnlineSubDirSeparator(&hierarchyDelimiter);
@@ -8428,17 +8472,18 @@ nsImapCacheStreamListener::OnStopRequest
     return NS_ERROR_NULL_POINTER;
   }
   nsresult rv = mListener->OnStopRequest(mChannelToUse, aStatus);
   nsCOMPtr<nsILoadGroup> loadGroup;
   mChannelToUse->GetLoadGroup(getter_AddRefs(loadGroup));
   if (loadGroup) loadGroup->RemoveRequest(mChannelToUse, nullptr, aStatus);
 
   mListener = nullptr;
-  mChannelToUse->Close();
+  nsresult rv3 = mChannelToUse->Close();
+  if (NS_FAILED(rv3)) NS_WARNING("mChannelToUse->Close() failed");
   mChannelToUse = nullptr;
   return rv;
 }
 
 NS_IMETHODIMP
 nsImapCacheStreamListener::OnDataAvailable(nsIRequest *request,
                                            nsIInputStream *aInStream,
                                            uint64_t aSourceOffset,
@@ -9022,29 +9067,32 @@ nsresult nsImapMockChannel::ReadFromMemC
   if (shouldUseCacheEntry) {
     nsCOMPtr<nsIInputStream> in;
     uint32_t readCount;
     rv = entry->OpenInputStream(0, getter_AddRefs(in));
     NS_ENSURE_SUCCESS(rv, rv);
     const int kFirstBlockSize = 100;
     char firstBlock[kFirstBlockSize + 1];
 
+    // XXX TODO we should check if we have read sizeof(firstBlock).
     // Note: This will not work for a cache2 disk cache.
     // (see bug 1302422 comment #4)
     rv = in->Read(firstBlock, sizeof(firstBlock), &readCount);
     NS_ENSURE_SUCCESS(rv, rv);
     firstBlock[kFirstBlockSize] = '\0';
     int32_t findPos =
         MsgFindCharInSet(nsDependentCString(firstBlock), ":\n\r", 0);
     // Check that the first line is a header line, i.e., with a ':' in it
     // Or that it begins with "From " because some IMAP servers allow that,
     // even though it's technically invalid.
     shouldUseCacheEntry = ((findPos != -1 && firstBlock[findPos] == ':') ||
                            !(strncmp(firstBlock, "From ", 5)));
-    in->Close();
+    nsresult rv3 = in->Close();
+    if (NS_FAILED(rv3)) NS_WARNING("in->Close() failed");
+    in = nullptr;
   }
 
   MOZ_LOG(IMAPCache, LogLevel::Debug,
           ("ReadFromMemCache(): After header check: shouldUseCacheEntry=%s",
            shouldUseCacheEntry ? "true" : "false"));
 
   if (shouldUseCacheEntry) {
     nsCOMPtr<nsIInputStream> in;
--- a/mailnews/imap/src/nsImapService.cpp
+++ b/mailnews/imap/src/nsImapService.cpp
@@ -1867,16 +1867,17 @@ NS_IMETHODIMP nsImapService::OnlineMessa
 
 nsresult nsImapService::OfflineAppendFromFile(
     nsIFile *aFile, nsIURI *aUrl, nsIMsgFolder *aDstFolder,
     const nsACString &messageId,  // to be replaced
     bool inSelectedState,         // needs to be in
     nsIUrlListener *aListener, nsIURI **aURL, nsISupports *aCopyState) {
   nsCOMPtr<nsIMsgDatabase> destDB;
   nsresult rv = aDstFolder->GetMsgDatabase(getter_AddRefs(destDB));
+  nsresult rv2 = NS_OK, rv3 = NS_OK, rv4 = NS_OK, rv5 = NS_OK;
   // ### might need to send some notifications instead of just returning
 
   bool isLocked;
   aDstFolder->GetLocked(&isLocked);
   if (isLocked) return NS_MSG_FOLDER_BUSY;
 
   if (NS_SUCCEEDED(rv) && destDB) {
     nsMsgKey fakeKey;
@@ -1941,16 +1942,23 @@ nsresult nsImapService::OfflineAppendFro
           char *newLine = nullptr;
           uint32_t numBytesInLine = 0;
           do {
             newLine = inputStreamBuffer->ReadNextLine(
                 inputStream, numBytesInLine, needMoreData);
             if (newLine) {
               msgParser->ParseAFolderLine(newLine, numBytesInLine);
               rv = offlineStore->Write(newLine, numBytesInLine, &bytesWritten);
+              // XXX TODO Check network errors, etc.
+              if (NS_FAILED(rv) || numBytesInLine != bytesWritten) {
+                // XXX / TODO probably we need to break out the loop on error.
+                MOZ_ASSERT(NS_SUCCEEDED(rv) && numBytesInLine == bytesWritten,
+                           "offlineStore->Write(newLine, numBytesInLine, "
+                           "&bytesWritten) failed");
+              }
               free(newLine);
             }
           } while (newLine);
           msgParser->FinishHeader();
 
           nsCOMPtr<nsIMsgDBHdr> fakeHdr;
           msgParser->GetNewMsgHdr(getter_AddRefs(fakeHdr));
           if (fakeHdr) {
@@ -1958,30 +1966,58 @@ nsresult nsImapService::OfflineAppendFro
               uint32_t resultFlags;
               fakeHdr->SetMessageOffset(curOfflineStorePos);
               fakeHdr->OrFlags(
                   nsMsgMessageFlags::Offline | nsMsgMessageFlags::Read,
                   &resultFlags);
               fakeHdr->SetOfflineMessageSize(fileSize);
               destDB->AddNewHdrToDB(fakeHdr, true /* notify */);
               aDstFolder->SetFlag(nsMsgFolderFlags::OfflineEvents);
-              if (msgStore) msgStore->FinishNewMessage(offlineStore, fakeHdr);
+              if (msgStore) {
+                rv2 = msgStore->FinishNewMessage(offlineStore, fakeHdr);
+                // FinishNewMessage now closes file stream always
+                offlineStore = nullptr;
+                if (NS_FAILED(rv2))
+                  NS_WARNING("msgStore->FinishNewMessage(...) failed");
+              }
             }
           }
           // tell the listener we're done.
-          inputStream->Close();
+          rv3 = inputStream->Close();
+          if (NS_FAILED(rv3)) NS_WARNING("inputStream->Close() failed");
           inputStream = nullptr;
           aListener->OnStopRunningUrl(aUrl, NS_OK);
         }
-        offlineStore->Close();
+        if (offlineStore) {
+          rv4 = offlineStore->Close();
+          if (NS_FAILED(rv4)) NS_WARNING("offlineStore->Close() failed");
+          offlineStore = nullptr;
+        }
       }
     }
   }
 
-  if (destDB) destDB->Close(true);
+  if (destDB) {
+    rv5 = destDB->Close(true);
+    if (NS_FAILED(rv5)) NS_WARNING("destDB->Close() failed");
+  } else {
+    NS_WARNING("destDB was null.");
+  }
+  // XXX The following just shows that the original code never
+  // was written with I/O errors in mind at all.
+
+  if (NS_FAILED(rv2))  // FinishMessage
+    return rv2;
+  // input stream close. Ignore for now.
+  // if (NS_FAILED(rv3))
+  //    return rv3;
+  if (NS_FAILED(rv4))  // offline->Close()
+    return rv4;
+  if (NS_FAILED(rv5))  // destDB->Close()
+    return rv5;
   return rv;
 }
 
 /* append message from file url */
 /* imap://HOST>appendmsgfromfile>DESTINATIONMAILBOXPATH */
 /* imap://HOST>appenddraftfromfile>DESTINATIONMAILBOXPATH>UID>messageId */
 NS_IMETHODIMP nsImapService::AppendMessageFromFile(
     nsIFile *aFile, nsIMsgFolder *aDstFolder,
--- a/mailnews/imap/src/nsImapUndoTxn.cpp
+++ b/mailnews/imap/src/nsImapUndoTxn.cpp
@@ -508,32 +508,40 @@ NS_IMETHODIMP nsImapOfflineTxn::UndoTran
           nsCOMPtr<nsIMsgDBHdr> undeletedHdr = m_srcHdrs[i];
           m_srcHdrs[i]->GetMessageKey(&msgKey);
           if (undeletedHdr) {
             nsCOMPtr<nsIMsgDBHdr> newHdr;
             srcDB->CopyHdrFromExistingHdr(msgKey, undeletedHdr, true,
                                           getter_AddRefs(newHdr));
           }
         }
-        srcDB->Close(true);
+        nsresult rv3 = srcDB->Close(true);
+        if (NS_FAILED(rv3)) NS_WARNING("srcDB->Close(true) failed");
+        srcDB = nullptr;
         srcFolder->SummaryChanged();
       }
       break;
     }
     case nsIMsgOfflineImapOperation::kMsgMarkedDeleted: {
       nsMsgKey msgKey;
       for (int32_t i = 0; i < m_srcHdrs.Count(); i++) {
         m_srcHdrs[i]->GetMessageKey(&msgKey);
         srcDB->MarkImapDeleted(msgKey, false, nullptr);
       }
     } break;
     default:
       break;
   }
-  srcDB->Close(true);
+  if (srcDB) {
+    nsresult rv3 = srcDB->Close(true);
+    if (NS_FAILED(rv3)) NS_WARNING("srcDB->Close(true) failed");
+    srcDB = nullptr;  // code uniformity
+  } else {
+    NS_WARNING("srcDB was null.");
+  }
   srcFolder->SummaryChanged();
   return NS_OK;
 }
 
 NS_IMETHODIMP nsImapOfflineTxn::RedoTransaction(void) {
   nsresult rv;
 
   nsCOMPtr<nsIMsgFolder> srcFolder = do_QueryReferent(m_srcFolder, &rv);
@@ -588,17 +596,23 @@ NS_IMETHODIMP nsImapOfflineTxn::RedoTran
         rv = destDB->GetOfflineOpForKey(msgKey, true, getter_AddRefs(op));
         if (NS_SUCCEEDED(rv) && op) {
           nsCString folderURI;
           srcFolder->GetURI(folderURI);
           op->SetSourceFolderURI(folderURI.get());
         }
       }
       dstFolder->SummaryChanged();
-      destDB->Close(true);
+      if (destDB) {
+        nsresult rv3 = destDB->Close(true);
+        if (NS_FAILED(rv3)) NS_WARNING("destDB->Close(true) failed");
+        destDB = nullptr;
+      } else {
+        NS_WARNING("destDB was nullptr.");
+      }
     } break;
     case nsIMsgOfflineImapOperation::kDeletedMsg:
       for (int32_t i = 0; i < m_srcHdrs.Count(); i++) {
         nsMsgKey msgKey;
         m_srcHdrs[i]->GetMessageKey(&msgKey);
         srcDB->DeleteMessage(msgKey, nullptr, true);
       }
       break;
@@ -622,13 +636,18 @@ NS_IMETHODIMP nsImapOfflineTxn::RedoTran
           else
             op->SetFlagOperation(newMsgFlags & ~m_flags);
         }
       }
       break;
     default:
       break;
   }
-  srcDB->Close(true);
-  srcDB = nullptr;
+  if (srcDB) {
+    nsresult rv3 = srcDB->Close(true);
+    if (NS_FAILED(rv3)) NS_WARNING("srcDB->Close(true) failed.");
+    srcDB = nullptr;  // code uniformity
+  } else {
+    NS_WARNING("srcDB was nullptr");
+  }
   srcFolder->SummaryChanged();
   return NS_OK;
 }