Bug 1242042: Enabling buffering for file stream to write message for C-C TB (Enabling buffering first step.) draft
authorISHIKAWA, Chiaki <ishikawa@yk.rim.or.jp>
Mon, 23 May 2022 03:03:51 +0900
changeset 116821 57eb169feb5ea5ed8b748bd4dd8a29fce2be1c8f
parent 116820 205420eec22cd0e2ae057771a6ab6007e907483d
child 116822 8fb8d1160e506d631182439d29c8f0a87db4db29
push id15955
push userishikawa@yk.rim.or.jp
push dateSun, 22 May 2022 18:04:18 +0000
treeherdertry-comm-central@beaacca97fbd [default view] [failures only]
bugs1242042
Bug 1242042: Enabling buffering for file stream to write message for C-C TB (Enabling buffering first step.)
mailnews/local/src/nsLocalMailFolder.cpp
--- a/mailnews/local/src/nsLocalMailFolder.cpp
+++ b/mailnews/local/src/nsLocalMailFolder.cpp
@@ -2016,17 +2016,78 @@ nsresult nsMsgLocalMailFolder::InitCopyM
   nsresult rv = GetMsgStore(getter_AddRefs(mCopyState->m_msgStore));
   NS_ENSURE_SUCCESS(rv, rv);
   rv = mCopyState->m_msgStore->GetNewMsgOutputStream(
       this, getter_AddRefs(mCopyState->m_newHdr),
       getter_AddRefs(mCopyState->m_fileStream));
   NS_ENSURE_SUCCESS(rv, rv);
   if (mCopyState->m_parseMsgState)
     mCopyState->m_parseMsgState->SetNewMsgHdr(mCopyState->m_newHdr);
-  return rv;
+
+  // sanity check
+  NS_ASSERTION(mCopyState->m_fileStream,
+               "mCopyState->m_fileStream should not be nullptr.");
+  // let us buffer the m_fileStream
+  // We Flush(), but this is for formality to
+  // get internal state right just in case.
+  // No error is expected and error is ignored.
+  mCopyState->m_fileStream->Flush();
+
+  nsCOMPtr<nsIOutputStream> tmp_in = mCopyState->m_fileStream;
+  nsCOMPtr<nsIOutputStream> tmp_out;
+
+  nsresult rv2 = NS_NewBufferedOutputStream(getter_AddRefs(tmp_out),
+                                            tmp_in.forget(), 64 * 1024);
+  mCopyState->m_fileStream = tmp_out;
+#ifdef DEBUG
+  fprintf(stderr,
+          "(debug) creating a buffered mCopyState->m_filestream in "
+          "nsMsgLocalMailFolder::InitCopyMsgHdrAndFileStream.\n");
+  NS_ASSERTION(NS_SUCCEEDED(rv2),
+               "Buffering of mCopyState->m_fileStream via "
+               "NS_NewBufferdOutputStream ought to succeed "
+              "for performance reason.\n");
+#endif
+  NS_ENSURE_SUCCESS(rv2, rv2);
+
+  // BIG PROBLEM we must fix.
+  // Turning the stream into a buffered stream REWINDS the file position to the beginning, 0.
+  // A surprising side effect.
+  // So we must move the file position again to the end.
+  // This was the cause of message corruption about a dozen years ago
+  // when buffered write was first attmpted if my observation is correct.
+  {
+    // create a seekable stream and move it to the end.
+    nsCOMPtr<nsISeekableStream> seekableStream =
+      do_QueryInterface(mCopyState->m_fileStream, &rv);
+#ifdef DEBUG
+    if (NS_FAILED(rv)) {
+      fprintf(stderr,"{debug} creating seekable stream failed: %s:%d\n",
+              __FILE__, __LINE__);
+    }
+#endif
+    NS_ENSURE_SUCCESS(rv, rv);
+    int64_t filepos1 = -1;
+    int64_t filepos2 = -1;
+    seekableStream->Tell(&filepos1);
+    rv = seekableStream->Seek(nsISeekableStream::NS_SEEK_END, 0);
+    seekableStream->Tell(&filepos2);
+#ifdef DEBUG
+    if (NS_FAILED(rv)) {
+      fprintf(stderr,"{debug} Seek to the end: %s:%d\n",
+              __FILE__, __LINE__);
+    } else {
+      fprintf(stderr, "{debug} fileposition before seek was %" PRIu64
+              " and afterward is = %" PRIu64 "\n",
+              filepos1, filepos2);
+    }
+#endif
+    NS_ENSURE_SUCCESS(rv, rv);
+  }
+  return NS_OK;  // error will return early.
 }
 
 // nsICopyMessageListener
 NS_IMETHODIMP nsMsgLocalMailFolder::BeginCopy() {
   if (!mCopyState) return NS_ERROR_NULL_POINTER;
 
   if (!mCopyState->m_copyingMultipleMessages) {
     nsresult rv = InitCopyMsgHdrAndFileStream();
@@ -2113,16 +2174,41 @@ NS_IMETHODIMP nsMsgLocalMailFolder::Copy
     char* newBuffer = (char*)PR_REALLOC(mCopyState->m_dataBuffer,
                                         aLength + mCopyState->m_leftOver + 4);
     if (!newBuffer) return NS_ERROR_OUT_OF_MEMORY;
 
     mCopyState->m_dataBuffer = newBuffer;
     mCopyState->m_dataBufferSize = aLength + mCopyState->m_leftOver + 3;
   }
 
+#if 0
+  // This seems superfulous as long as |CopyData()| is always called
+  // after |InitCopyMsgHdrAndFileStream()|, where m_fileStream is
+  // buffered, has been invoked.
+
+  //
+  // Flush before seek.
+  // Enable buffering for Write.
+  //
+
+  // sanity check first.
+  NS_ASSERTION(mCopyState->m_fileStream,
+               "mCopyState->m_fileStream should not be null.");
+
+  // just in case. Ignore error here.
+  mCopyState->m_fileStream->Flush(); 
+  mCopyState->m_fileStream = NS_BufferOutputStream(mCopyState->m_fileStream,
+                                                   64 * 1024 );
+#  ifdef DEBUG
+  fprintf(stderr,
+          "(debug) creating a buffered mCopyState->m_filestream in "
+          "nsMsgLocalMailFolder::CopyData().\n");
+#  endif
+#endif
+
   // BTW, under Linux, and for POP3 case, Read is handled by recvfrom in
   // lower-level code with 32KB buffer: verified from strace data
 
   rv = aIStream->Read(mCopyState->m_dataBuffer + mCopyState->m_leftOver + 1,
                       aLength, &readCount);
   NS_ENSURE_SUCCESS(rv, rv);
   mCopyState->m_leftOver += readCount;
   mCopyState->m_dataBuffer[mCopyState->m_leftOver + 1] = '\0';