Bug 928321 - Implement a variant of safe-file-output-stream that doesn't flush by default. r=Yoric
authorSumit Agrawal <sumit4iit@gmail.com>
Mon, 02 Dec 2013 12:51:25 -0500
changeset 172991 f66fa95ae065213f4c453c906617d4093b23ae7e
parent 172990 2716ba96528cdca563b9bf5390b2d1fea4327a50
child 172992 0ec4cc06f49f80deaaafcfd0260f126dbd94a8ce
push id3224
push userlsblakk@mozilla.com
push dateTue, 04 Feb 2014 01:06:49 +0000
treeherdermozilla-beta@60c04d0987f1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersYoric
bugs928321
milestone28.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 928321 - Implement a variant of safe-file-output-stream that doesn't flush by default. r=Yoric
netwerk/base/public/nsNetUtil.h
netwerk/base/src/nsFileStreams.cpp
netwerk/base/src/nsFileStreams.h
netwerk/build/nsNetCID.h
netwerk/build/nsNetModule.cpp
netwerk/test/unit/test_safeoutputstream.js
--- a/netwerk/base/public/nsNetUtil.h
+++ b/netwerk/base/public/nsNetUtil.h
@@ -994,16 +994,35 @@ NS_NewLocalFileOutputStream(nsIOutputStr
         if (NS_SUCCEEDED(rv))
             out.forget(result);
     }
     return rv;
 }
 
 // returns a file output stream which can be QI'ed to nsISafeOutputStream.
 inline nsresult
+NS_NewAtomicFileOutputStream(nsIOutputStream **result,
+                                nsIFile          *file,
+                                int32_t           ioFlags       = -1,
+                                int32_t           perm          = -1,
+                                int32_t           behaviorFlags = 0)
+{
+    nsresult rv;
+    nsCOMPtr<nsIFileOutputStream> out =
+        do_CreateInstance(NS_ATOMICLOCALFILEOUTPUTSTREAM_CONTRACTID, &rv);
+    if (NS_SUCCEEDED(rv)) {
+        rv = out->Init(file, ioFlags, perm, behaviorFlags);
+        if (NS_SUCCEEDED(rv))
+            out.forget(result);
+    }
+    return rv;
+}
+
+// returns a file output stream which can be QI'ed to nsISafeOutputStream.
+inline nsresult
 NS_NewSafeLocalFileOutputStream(nsIOutputStream **result,
                                 nsIFile          *file,
                                 int32_t           ioFlags       = -1,
                                 int32_t           perm          = -1,
                                 int32_t           behaviorFlags = 0)
 {
     nsresult rv;
     nsCOMPtr<nsIFileOutputStream> out =
--- a/netwerk/base/src/nsFileStreams.cpp
+++ b/netwerk/base/src/nsFileStreams.cpp
@@ -827,33 +827,33 @@ nsFileOutputStream::Init(nsIFile* file, 
     if (perm <= 0)
         perm = 0664;
 
     return MaybeOpen(file, ioFlags, perm,
                      mBehaviorFlags & nsIFileOutputStream::DEFER_OPEN);
 }
 
 ////////////////////////////////////////////////////////////////////////////////
-// nsSafeFileOutputStream
+// nsAtomicFileOutputStream
 
-NS_IMPL_ISUPPORTS_INHERITED3(nsSafeFileOutputStream,
+NS_IMPL_ISUPPORTS_INHERITED3(nsAtomicFileOutputStream,
                              nsFileOutputStream,
                              nsISafeOutputStream,
                              nsIOutputStream,
                              nsIFileOutputStream)
 
 NS_IMETHODIMP
-nsSafeFileOutputStream::Init(nsIFile* file, int32_t ioFlags, int32_t perm,
+nsAtomicFileOutputStream::Init(nsIFile* file, int32_t ioFlags, int32_t perm,
                              int32_t behaviorFlags)
 {
     return nsFileOutputStream::Init(file, ioFlags, perm, behaviorFlags);
 }
 
 nsresult
-nsSafeFileOutputStream::DoOpen()
+nsAtomicFileOutputStream::DoOpen()
 {
     // Make sure mOpenParams.localFile will be empty if we bail somewhere in
     // this function
     nsCOMPtr<nsIFile> file;
     file.swap(mOpenParams.localFile);
 
     nsresult rv = file->Exists(&mTargetFileExists);
     if (NS_FAILED(rv)) {
@@ -891,51 +891,50 @@ nsSafeFileOutputStream::DoOpen()
         mTempFile = tempResult;
         mTargetFile = file;
         rv = nsFileOutputStream::DoOpen();
     }
     return rv;
 }
 
 NS_IMETHODIMP
-nsSafeFileOutputStream::Close()
+nsAtomicFileOutputStream::Close()
 {
     nsresult rv = nsFileOutputStream::Close();
 
     // the consumer doesn't want the original file overwritten -
     // so clean up by removing the temp file.
     if (mTempFile) {
         mTempFile->Remove(false);
         mTempFile = nullptr;
     }
 
     return rv;
 }
 
 NS_IMETHODIMP
-nsSafeFileOutputStream::Finish()
+nsAtomicFileOutputStream::Finish()
 {
-    Flush();
     nsresult rv = nsFileOutputStream::Close();
 
     // if there is no temp file, don't try to move it over the original target.
     // It would destroy the targetfile if close() is called twice.
     if (!mTempFile)
         return rv;
 
     // Only overwrite if everything was ok, and the temp file could be closed.
     if (NS_SUCCEEDED(mWriteResult) && NS_SUCCEEDED(rv)) {
         NS_ENSURE_STATE(mTargetFile);
 
         if (!mTargetFileExists) {
             // If the target file did not exist when we were initialized, then the
             // temp file we gave out was actually a reference to the target file.
             // since we succeeded in writing to the temp file (and hence succeeded
             // in writing to the target file), there is nothing more to do.
-#ifdef DEBUG      
+#ifdef DEBUG
             bool equal;
             if (NS_FAILED(mTargetFile->Equals(mTempFile, &equal)) || !equal)
                 NS_ERROR("mTempFile not equal to mTargetFile");
 #endif
         }
         else {
             nsAutoString targetFilename;
             rv = mTargetFile->GetLeafName(targetFilename);
@@ -954,32 +953,42 @@ nsSafeFileOutputStream::Finish()
         if (NS_FAILED(mWriteResult))
             rv = mWriteResult;
     }
     mTempFile = nullptr;
     return rv;
 }
 
 NS_IMETHODIMP
-nsSafeFileOutputStream::Write(const char *buf, uint32_t count, uint32_t *result)
+nsAtomicFileOutputStream::Write(const char *buf, uint32_t count, uint32_t *result)
 {
     nsresult rv = nsFileOutputStream::Write(buf, count, result);
     if (NS_SUCCEEDED(mWriteResult)) {
         if (NS_FAILED(rv))
             mWriteResult = rv;
         else if (count != *result)
             mWriteResult = NS_ERROR_LOSS_OF_SIGNIFICANT_DATA;
 
         if (NS_FAILED(mWriteResult) && count > 0)
             NS_WARNING("writing to output stream failed! data may be lost");
-    } 
+    }
     return rv;
 }
 
 ////////////////////////////////////////////////////////////////////////////////
+// nsSafeFileOutputStream
+
+NS_IMETHODIMP
+nsSafeFileOutputStream::Finish()
+{
+    (void) Flush();
+    return nsAtomicFileOutputStream::Finish();
+}
+
+////////////////////////////////////////////////////////////////////////////////
 // nsFileStream
 
 NS_IMPL_ISUPPORTS_INHERITED3(nsFileStream,
                              nsFileStreamBase,
                              nsIInputStream,
                              nsIOutputStream,
                              nsIFileStream)
 
--- a/netwerk/base/src/nsFileStreams.h
+++ b/netwerk/base/src/nsFileStreams.h
@@ -220,44 +220,65 @@ public:
     }
 
     static nsresult
     Create(nsISupports *aOuter, REFNSIID aIID, void **aResult);
 };
 
 ////////////////////////////////////////////////////////////////////////////////
 
-class nsSafeFileOutputStream : public nsFileOutputStream,
-                               public nsISafeOutputStream
+/**
+ * A safe file output stream that overwrites the destination file only
+ * once writing is complete. This protects against incomplete writes
+ * due to the process or the thread being interrupted or crashed.
+ */
+class nsAtomicFileOutputStream : public nsFileOutputStream,
+                                 public nsISafeOutputStream
 {
 public:
     NS_DECL_ISUPPORTS_INHERITED
     NS_DECL_NSISAFEOUTPUTSTREAM
 
-    nsSafeFileOutputStream() :
+    nsAtomicFileOutputStream() :
         mTargetFileExists(true),
         mWriteResult(NS_OK) {}
 
-    virtual ~nsSafeFileOutputStream()
+    virtual ~nsAtomicFileOutputStream()
     {
         Close();
     }
 
-    virtual nsresult DoOpen();
+    virtual nsresult DoOpen() MOZ_OVERRIDE;
 
     NS_IMETHODIMP Close();
     NS_IMETHODIMP Write(const char *buf, uint32_t count, uint32_t *result);
     NS_IMETHODIMP Init(nsIFile* file, int32_t ioFlags, int32_t perm, int32_t behaviorFlags);
 
 protected:
     nsCOMPtr<nsIFile>         mTargetFile;
     nsCOMPtr<nsIFile>         mTempFile;
 
     bool     mTargetFileExists;
     nsresult mWriteResult; // Internally set in Write()
+
+};
+
+////////////////////////////////////////////////////////////////////////////////
+
+/**
+ * A safe file output stream that overwrites the destination file only
+ * once writing + flushing is complete. This protects against more
+ * classes of software/hardware errors than nsAtomicFileOutputStream,
+ * at the expense of being more costly to the disk, OS and battery.
+ */
+class nsSafeFileOutputStream : public nsAtomicFileOutputStream
+{
+public:
+
+    NS_IMETHOD Finish();
 };
 
 ////////////////////////////////////////////////////////////////////////////////
 
 class nsFileStream : public nsFileStreamBase,
                      public nsIInputStream,
                      public nsIOutputStream,
                      public nsIFileStream
--- a/netwerk/build/nsNetCID.h
+++ b/netwerk/build/nsNetCID.h
@@ -391,17 +391,27 @@
 #define NS_BUFFEREDOUTPUTSTREAM_CID                  \
 { /* 9868b4ce-da08-11d3-8cda-0060b0fc14a3 */         \
     0x9868b4ce,                                      \
     0xda08,                                          \
     0x11d3,                                          \
     {0x8c, 0xda, 0x00, 0x60, 0xb0, 0xfc, 0x14, 0xa3} \
 }
 
-// component implementing nsISafeOutputStream
+// components implementing nsISafeOutputStream
+#define NS_ATOMICLOCALFILEOUTPUTSTREAM_CONTRACTID \
+    "@mozilla.org/network/atomic-file-output-stream;1"
+#define NS_ATOMICLOCALFILEOUTPUTSTREAM_CID           \
+{ /* 6EAE857E-4BA9-11E3-9B39-B4036188709B */         \
+    0x6EAE857E,                                      \
+    0x4BA9,                                          \
+    0x11E3,                                          \
+    {0x9b, 0x39, 0xb4, 0x03, 0x61, 0x88, 0x70, 0x9b} \
+}
+
 #define NS_SAFELOCALFILEOUTPUTSTREAM_CONTRACTID \
     "@mozilla.org/network/safe-file-output-stream;1"
 #define NS_SAFELOCALFILEOUTPUTSTREAM_CID             \
 { /* a181af0d-68b8-4308-94db-d4f859058215 */         \
     0xa181af0d,                                      \
     0x68b8,                                          \
     0x4308,                                          \
     {0x94, 0xdb, 0xd4, 0xf8, 0x59, 0x05, 0x82, 0x15} \
--- a/netwerk/build/nsNetModule.cpp
+++ b/netwerk/build/nsNetModule.cpp
@@ -97,16 +97,18 @@ namespace net {
 NS_GENERIC_FACTORY_CONSTRUCTOR_INIT(BackgroundFileSaverOutputStream, Init)
 NS_GENERIC_FACTORY_CONSTRUCTOR_INIT(BackgroundFileSaverStreamListener, Init)
 } // namespace net
 } // namespace mozilla
 
 #include "nsSyncStreamListener.h"
 NS_GENERIC_FACTORY_CONSTRUCTOR_INIT(nsSyncStreamListener, Init)
 
+NS_GENERIC_FACTORY_CONSTRUCTOR(nsAtomicFileOutputStream)
+
 NS_GENERIC_FACTORY_CONSTRUCTOR(nsSafeFileOutputStream)
 
 NS_GENERIC_FACTORY_CONSTRUCTOR(nsFileStream)
 
 NS_GENERIC_AGGREGATED_CONSTRUCTOR_INIT(nsLoadGroup, Init)
 
 #include "ArrayBufferInputStream.h"
 NS_GENERIC_FACTORY_CONSTRUCTOR(ArrayBufferInputStream)
@@ -700,16 +702,17 @@ NS_DEFINE_NAMED_CID(NS_BACKGROUNDFILESAV
 NS_DEFINE_NAMED_CID(NS_SYNCSTREAMLISTENER_CID);
 NS_DEFINE_NAMED_CID(NS_REQUESTOBSERVERPROXY_CID);
 NS_DEFINE_NAMED_CID(NS_SIMPLESTREAMLISTENER_CID);
 NS_DEFINE_NAMED_CID(NS_STREAMLISTENERTEE_CID);
 NS_DEFINE_NAMED_CID(NS_LOADGROUP_CID);
 NS_DEFINE_NAMED_CID(NS_LOCALFILEINPUTSTREAM_CID);
 NS_DEFINE_NAMED_CID(NS_LOCALFILEOUTPUTSTREAM_CID);
 NS_DEFINE_NAMED_CID(NS_PARTIALLOCALFILEINPUTSTREAM_CID);
+NS_DEFINE_NAMED_CID(NS_ATOMICLOCALFILEOUTPUTSTREAM_CID);
 NS_DEFINE_NAMED_CID(NS_SAFELOCALFILEOUTPUTSTREAM_CID);
 NS_DEFINE_NAMED_CID(NS_LOCALFILESTREAM_CID);
 NS_DEFINE_NAMED_CID(NS_URICHECKER_CID);
 NS_DEFINE_NAMED_CID(NS_INCREMENTALDOWNLOAD_CID);
 NS_DEFINE_NAMED_CID(NS_STDURLPARSER_CID);
 NS_DEFINE_NAMED_CID(NS_NOAUTHURLPARSER_CID);
 NS_DEFINE_NAMED_CID(NS_AUTHURLPARSER_CID);
 NS_DEFINE_NAMED_CID(NS_STANDARDURL_CID);
@@ -840,16 +843,17 @@ static const mozilla::Module::CIDEntry k
     { &kNS_SYNCSTREAMLISTENER_CID, false, nullptr, nsSyncStreamListenerConstructor },
     { &kNS_REQUESTOBSERVERPROXY_CID, false, nullptr, nsRequestObserverProxyConstructor },
     { &kNS_SIMPLESTREAMLISTENER_CID, false, nullptr, nsSimpleStreamListenerConstructor },
     { &kNS_STREAMLISTENERTEE_CID, false, nullptr, nsStreamListenerTeeConstructor },
     { &kNS_LOADGROUP_CID, false, nullptr, nsLoadGroupConstructor },
     { &kNS_LOCALFILEINPUTSTREAM_CID, false, nullptr, nsFileInputStream::Create },
     { &kNS_LOCALFILEOUTPUTSTREAM_CID, false, nullptr, nsFileOutputStream::Create },
     { &kNS_PARTIALLOCALFILEINPUTSTREAM_CID, false, nullptr, nsPartialFileInputStream::Create },
+    { &kNS_ATOMICLOCALFILEOUTPUTSTREAM_CID, false, nullptr, nsAtomicFileOutputStreamConstructor },
     { &kNS_SAFELOCALFILEOUTPUTSTREAM_CID, false, nullptr, nsSafeFileOutputStreamConstructor },
     { &kNS_LOCALFILESTREAM_CID, false, nullptr, nsFileStreamConstructor },
     { &kNS_URICHECKER_CID, false, nullptr, nsURICheckerConstructor },
     { &kNS_INCREMENTALDOWNLOAD_CID, false, nullptr, net_NewIncrementalDownload },
     { &kNS_STDURLPARSER_CID, false, nullptr, nsStdURLParserConstructor },
     { &kNS_NOAUTHURLPARSER_CID, false, nullptr, nsNoAuthURLParserConstructor },
     { &kNS_AUTHURLPARSER_CID, false, nullptr, nsAuthURLParserConstructor },
     { &kNS_STANDARDURL_CID, false, nullptr, nsStandardURLConstructor },
@@ -982,16 +986,17 @@ static const mozilla::Module::ContractID
     { NS_SYNCSTREAMLISTENER_CONTRACTID, &kNS_SYNCSTREAMLISTENER_CID },
     { NS_REQUESTOBSERVERPROXY_CONTRACTID, &kNS_REQUESTOBSERVERPROXY_CID },
     { NS_SIMPLESTREAMLISTENER_CONTRACTID, &kNS_SIMPLESTREAMLISTENER_CID },
     { NS_STREAMLISTENERTEE_CONTRACTID, &kNS_STREAMLISTENERTEE_CID },
     { NS_LOADGROUP_CONTRACTID, &kNS_LOADGROUP_CID },
     { NS_LOCALFILEINPUTSTREAM_CONTRACTID, &kNS_LOCALFILEINPUTSTREAM_CID },
     { NS_LOCALFILEOUTPUTSTREAM_CONTRACTID, &kNS_LOCALFILEOUTPUTSTREAM_CID },
     { NS_PARTIALLOCALFILEINPUTSTREAM_CONTRACTID, &kNS_PARTIALLOCALFILEINPUTSTREAM_CID },
+    { NS_ATOMICLOCALFILEOUTPUTSTREAM_CONTRACTID, &kNS_ATOMICLOCALFILEOUTPUTSTREAM_CID },
     { NS_SAFELOCALFILEOUTPUTSTREAM_CONTRACTID, &kNS_SAFELOCALFILEOUTPUTSTREAM_CID },
     { NS_LOCALFILESTREAM_CONTRACTID, &kNS_LOCALFILESTREAM_CID },
     { NS_URICHECKER_CONTRACT_ID, &kNS_URICHECKER_CID },
     { NS_INCREMENTALDOWNLOAD_CONTRACTID, &kNS_INCREMENTALDOWNLOAD_CID },
     { NS_STDURLPARSER_CONTRACTID, &kNS_STDURLPARSER_CID },
     { NS_NOAUTHURLPARSER_CONTRACTID, &kNS_NOAUTHURLPARSER_CID },
     { NS_AUTHURLPARSER_CONTRACTID, &kNS_AUTHURLPARSER_CID },
     { NS_STANDARDURL_CONTRACTID, &kNS_STANDARDURL_CID },
--- a/netwerk/test/unit/test_safeoutputstream.js
+++ b/netwerk/test/unit/test_safeoutputstream.js
@@ -1,13 +1,27 @@
 /* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
 /* 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/. */
 
+function write_atomic(file, str) {
+    var stream = Cc["@mozilla.org/network/atomic-file-output-stream;1"]
+                   .createInstance(Ci.nsIFileOutputStream);
+    stream.init(file, -1, -1, 0);
+    do {
+        var written = stream.write(str, str.length);
+        if (written == str.length)
+            break;
+        str = str.substring(written);
+    } while (1);
+    stream.QueryInterface(Ci.nsISafeOutputStream).finish();
+    stream.close();
+}
+
 function write(file, str) {
     var stream = Cc["@mozilla.org/network/safe-file-output-stream;1"]
                    .createInstance(Ci.nsIFileOutputStream);
     stream.init(file, -1, -1, 0);
     do {
         var written = stream.write(str, str.length);
         if (written == str.length)
             break;
@@ -38,9 +52,15 @@ function run_test()
                  .get("TmpD", Ci.nsIFile);
     file.append(filename);
 
     write(file, "First write");
     checkFile(file, "First write");
 
     write(file, "Second write");
     checkFile(file, "Second write");
+
+    write_atomic(file, "First write: Atomic");
+    checkFile(file, "First write: Atomic");
+
+    write_atomic(file, "Second write: Atomic");
+    checkFile(file, "Second write: Atomic");
 }