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 481669 a299ae02fc4c0a7c507e4410c28414a9fdb25b90
parent 481668 398ccedc20dc1c3f29332dd5a4791b9ab96eb547
child 481670 c33114c93dd0ebff53e29c92145b2673c496c339
push id9719
push userffxbld-merge
push dateFri, 24 Aug 2018 17:49:46 +0000
treeherdermozilla-beta@719ec98fba77 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerserahm
bugs1475612
milestone63.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 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;
 }