Bug 1475612: Fix double file close on background thread. r=erahm
authorKris Maglione <maglione.k@gmail.com>
Thu, 12 Jul 2018 23:13:04 -0700
changeset 474780 a299ae02fc4c0a7c507e4410c28414a9fdb25b90
parent 474779 398ccedc20dc1c3f29332dd5a4791b9ab96eb547
child 474781 c33114c93dd0ebff53e29c92145b2673c496c339
push id204
push userfmarier@mozilla.com
push dateWed, 25 Jul 2018 00:48:09 +0000
reviewerserahm
bugs1475612
milestone63.0a1
Bug 1475612: Fix double file close on background thread. r=erahm LSBUtils closes a file descriptor twice, once with fclose and then again with close. It also does this on a background thread, during startup, which means it tends to race with main thread code which opens files. This patch fixes that, and also removes a work-around for the issue in the MemMapSnapshot code. MozReview-Commit-ID: JdDHt9ayFEl
dom/ipc/MemMapSnapshot.cpp
widget/LSBUtils.cpp
--- a/dom/ipc/MemMapSnapshot.cpp
+++ b/dom/ipc/MemMapSnapshot.cpp
@@ -113,29 +113,17 @@ MemMapSnapshot::Freeze(AutoMemMap& aMem)
   // since we open and share a read-only file descriptor, and then delete the
   // file. But it doesn't hurt, either.
   chmod(mPath.get(), 0400);
 
   nsCOMPtr<nsIFile> file;
   MOZ_TRY(NS_NewNativeLocalFile(mPath, /* followLinks = */ false,
                                 getter_AddRefs(file)));
 
-  auto result = aMem.init(file);
-#ifdef XP_LINUX
-  // On Linux automation runs, every few hundred thousand calls, our attempt to
-  // stat the file that we just successfully opened fails with EBADF (bug
-  // 1472889). Presumably this is a race with a background thread that double
-  // closes a file, but is difficult to diagnose, so work around it by making a
-  // second mapping attempt if the first one fails.
-  if (!result.isOk()) {
-    aMem.reset();
-    result = aMem.init(file);
-  }
-#endif
-  return result;
+  return aMem.init(file);
 }
 
 #else
 #  error "Unsupported build configuration"
 #endif
 
 } // namespace ipc
 } // namespace mozilla
--- a/widget/LSBUtils.cpp
+++ b/widget/LSBUtils.cpp
@@ -3,16 +3,17 @@
  * 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 "LSBUtils.h"
 
 #include <unistd.h>
 #include "base/process_util.h"
+#include "mozilla/FileUtils.h"
 
 namespace mozilla {
 namespace widget {
 namespace lsb {
 
 static const char* gLsbReleasePath = "/usr/bin/lsb_release";
 
 bool
@@ -42,37 +43,33 @@ GetLSBRelease(nsACString& aDistributor,
   bool ok = base::LaunchApp(argv, options, &process);
   close(pipefd[1]);
   if (!ok) {
     NS_WARNING("Failed to spawn lsb_release!");
     close(pipefd[0]);
     return false;
   }
 
-  FILE* stream = fdopen(pipefd[0], "r");
+  ScopedCloseFile stream(fdopen(pipefd[0], "r"));
   if (!stream) {
     NS_WARNING("Could not wrap fd!");
     close(pipefd[0]);
     return false;
   }
 
   char dist[256], desc[256], release[256], codename[256];
   if (fscanf(stream, "Distributor ID:\t%255[^\n]\n"
                      "Description:\t%255[^\n]\n"
                      "Release:\t%255[^\n]\n"
                      "Codename:\t%255[^\n]\n",
              dist, desc, release, codename) != 4)
   {
     NS_WARNING("Failed to parse lsb_release!");
-    fclose(stream);
-    close(pipefd[0]);
     return false;
   }
-  fclose(stream);
-  close(pipefd[0]);
 
   aDistributor.Assign(dist);
   aDescription.Assign(desc);
   aRelease.Assign(release);
   aCodename.Assign(codename);
   return true;
 }