--- a/mailnews/imap/src/nsImapIncomingServer.cpp
+++ b/mailnews/imap/src/nsImapIncomingServer.cpp
@@ -585,19 +585,22 @@ nsresult nsImapIncomingServer::DoomUrlIf
mockChannel) {
nsresult requestStatus;
mockChannel->GetStatus(&requestStatus);
if (NS_FAILED(requestStatus)) {
nsresult res;
*urlDoomed = true;
nsImapProtocol::LogImapUrl("dooming url", aImapUrl);
- mockChannel
+ 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,22 @@ 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);
@@ -3016,20 +3019,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;
@@ -3053,17 +3059,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;
@@ -3105,17 +3120,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);
@@ -4121,29 +4139,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;
@@ -4229,24 +4256,32 @@ 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
@@ -4366,19 +4401,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);
}
@@ -5618,17 +5658,19 @@ 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) {
@@ -5646,17 +5688,19 @@ 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;
}
@@ -6395,25 +6439,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,
@@ -6501,17 +6549,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)
@@ -6622,24 +6670,44 @@ 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);
+ {
+ // 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
@@ -6665,17 +6733,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);
@@ -6705,17 +6773,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");
+ }
if (isMove) sourceMailDB->Commit(nsMsgDBCommitType::kLargeCommit);
database->Commit(nsMsgDBCommitType::kLargeCommit);
SummaryChanged();
srcFolder->SummaryChanged();
}
if (txnMgr) txnMgr->EndBatch(false);
}
@@ -6899,17 +6974,18 @@ 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.
@@ -7638,27 +7714,41 @@ 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)
@@ -7670,35 +7760,50 @@ 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");
+ }
+ // 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) {
@@ -8031,19 +8136,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");
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,37 @@ 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
@@ -1,9 +1,9 @@
-/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
// as does this
#include "msgCore.h" // for pre-compiled headers
#include "nsMsgUtils.h"
@@ -993,21 +993,26 @@ 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!
@@ -1111,17 +1116,22 @@ 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");
@@ -1143,24 +1153,28 @@ 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.
@@ -1292,16 +1306,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;
@@ -1705,23 +1720,38 @@ 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() &
@@ -1957,18 +1987,24 @@ 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);
@@ -3704,16 +3740,21 @@ 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())
@@ -5969,17 +6010,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);
@@ -6055,17 +6096,24 @@ 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);
@@ -8438,17 +8486,19 @@ 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,
@@ -9035,29 +9085,33 @@ 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
@@ -1940,16 +1940,21 @@ 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) {
+ 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) {
@@ -1957,30 +1962,50 @@ 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) {
+ nsresult 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();
+ nsresult rv3 = inputStream->Close();
+ if (NS_FAILED(rv3))
+ NS_WARNING("inputStream->Close() failed");
inputStream = nullptr;
aListener->OnStopRunningUrl(aUrl, NS_OK);
}
- offlineStore->Close();
+ if (offlineStore) {
+ nsresult rv2 = offlineStore->Close();
+ if (NS_FAILED(rv2))
+ NS_WARNING("offlineStore->Close() failed");
+ offlineStore = nullptr;
+ }
}
}
}
- if (destDB) destDB->Close(true);
+ if (destDB) {
+ nsresult rv3 = destDB->Close(true);
+ if (NS_FAILED(rv3))
+ NS_WARNING("destDB->Close() failed");
+ } else {
+ NS_WARNING("destDB was null.");
+ }
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
@@ -1,9 +1,9 @@
-/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
+/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
#include "msgCore.h" // for precompiled headers
#include "nsMsgImapCID.h"
#include "nsIMsgHdr.h"
#include "nsImapUndoTxn.h"
@@ -508,32 +508,42 @@ 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 +598,24 @@ 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 +639,19 @@ 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;
}