Bug 936987 - Propagate the error code of PR_Close() against a file under CIFS-share under Linux. r=froydnj
authorISHIKAWA, Chiaki <ishikawa@yk.rim.or.jp>
Tue, 19 Nov 2013 14:56:58 -0500
changeset 156440 a2af190cd480187aa9cf47e9da82d935148d4570
parent 156439 59994113d911f74824d9e3692596452a657f3dd1
child 156441 bbf4e009ba00199c5ecfbcb5ee3c25e1cb465391
push id25678
push userryanvm@gmail.com
push dateWed, 20 Nov 2013 03:26:13 +0000
treeherdermozilla-central@4f993fa378eb [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfroydnj
bugs936987
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 936987 - Propagate the error code of PR_Close() against a file under CIFS-share under Linux. r=froydnj (Network error causes the file share to fail, and read()/close() and possibly write() can return network-related error. close() error was not caught before.)
xpcom/io/nsLocalFileUnix.cpp
--- a/xpcom/io/nsLocalFileUnix.cpp
+++ b/xpcom/io/nsLocalFileUnix.cpp
@@ -1,9 +1,9 @@
-/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */ 
+/* -*- Mode: C++; tab-width: 4; 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/. */
 
 /**
  * Implementation of nsIFile for "unixy" systems.
  */
 
@@ -811,20 +811,22 @@ nsLocalFile::CopyToNative(nsIFile *newPa
         }
 
 #ifdef DEBUG_blizzard
         int32_t totalRead = 0;
         int32_t totalWritten = 0;
 #endif
         char buf[BUFSIZ];
         int32_t bytesRead;
-        
+
         // record PR_Write() error for better error message later.
-        nsresult saved_write_error = NS_OK;    
+        nsresult saved_write_error = NS_OK;
         nsresult saved_read_error = NS_OK;
+        nsresult saved_read_close_error = NS_OK;
+        nsresult saved_write_close_error = NS_OK;
 
         // DONE: Does PR_Read() return bytesRead < 0 for error?
         // Yes., The errors from PR_Read are not so common and
         // the value may not have correspondence in NS_ERROR_*, but
         // we do catch it still, immediately after while() loop.
         // We can differentiate errors pf PR_Read and PR_Write by
         // looking at saved_write_error value. If PR_Write error occurs (and not
         // PR_Read() error), save_write_error is not NS_OK.
@@ -843,49 +845,72 @@ nsLocalFile::CopyToNative(nsIFile *newPa
             }
             NS_ASSERTION(bytesWritten == bytesRead, "short PR_Write?");
 
 #ifdef DEBUG_blizzard
             totalWritten += bytesWritten;
 #endif
         }
 
+        // TODO/FIXME: If CIFS (and NFS?) may force read/write to return EINTR,
+        // we are better off to prepare for retrying. But we need confirmation if
+        // EINTR is returned.
+
         // Record error if PR_Read() failed.
         // Must be done before any other I/O which may reset errno.
         if ( (bytesRead < 0) && (saved_write_error == NS_OK)) {
             saved_read_error = NSRESULT_FOR_ERRNO();
         }
 
 #ifdef DEBUG_blizzard
         printf("read %d bytes, wrote %d bytes\n",
                  totalRead, totalWritten);
 #endif
 
-        // TODO/FIXME: better find out how to propagate errors of
-        // close.  Errors of close can occur.  Read man page of
-        // close(2); At least, we should tell the user that
-        // filesystem/disk is hosed so that users can take remedial
-        // action.
+        // DONE: Errors of close can occur.  Read man page of
+        // close(2);
+        // This is likely to happen if the file system is remote file
+        // system (NFS, CIFS, etc.) and network outage occurs.
+        // At least, we should tell the user that filesystem/disk is
+        // hosed (possibly due to network error, hard disk failure,
+        // etc.) so that users can take remedial action.
 
         // close the files
-        PR_Close(newFD);
-        PR_Close(oldFD);
+        if (PR_Close(newFD) < 0) {
+            saved_write_close_error = NSRESULT_FOR_ERRNO();
+#if DEBUG
+            // This error merits printing.
+            fprintf(stderr, "ERROR: PR_Close(newFD) returned error. errno = %d\n", errno);
+#endif
+        }
+
+        if (PR_Close(oldFD) < 0) {
+            saved_read_close_error = NSRESULT_FOR_ERRNO();
+#if DEBUG
+            fprintf(stderr, "ERROR: PR_Close(oldFD) returned error. errno = %d\n", errno);
+#endif
+        }
 
         // Let us report the failure to write and read.
         // check for write/read error after cleaning up
         if (bytesRead < 0) {
             if (saved_write_error != NS_OK)
                 return saved_write_error;
             else if (saved_read_error != NS_OK)
                 return saved_read_error;
 #if DEBUG
             else                // sanity check. Die and debug.
                 MOZ_ASSERT(0);
 #endif
         }
+
+        if (saved_write_close_error != NS_OK)
+            return saved_write_close_error;
+        if (saved_read_close_error != NS_OK)
+            return saved_read_close_error;
     }
     return rv;
 }
 
 NS_IMETHODIMP
 nsLocalFile::CopyToFollowingLinksNative(nsIFile *newParent, const nsACString &newName)
 {
     return CopyToNative(newParent, newName);