Bug 1237219 - Lock patch files while applying updates; r=rstrong
authorMatt Howell <mhowell@mozilla.com>
Thu, 14 Apr 2016 09:33:42 -0700
changeset 297069 3fe6c728ce7674ff710309587eaae01ca46e887d
parent 297068 4e78d7772c3e61431e3e4b0e9d9647863ab0a84e
child 297070 9bffb578e423f0e8eea97af5409557205f20ad9b
push id76587
push usercbook@mozilla.com
push dateThu, 12 May 2016 04:00:41 +0000
treeherdermozilla-inbound@9bffb578e423 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersrstrong
bugs1237219
milestone49.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 1237219 - Lock patch files while applying updates; r=rstrong MozReview-Commit-ID: 8m4SQVhZy4D
toolkit/mozapps/update/common/errors.h
toolkit/mozapps/update/updater/updater.cpp
--- a/toolkit/mozapps/update/common/errors.h
+++ b/toolkit/mozapps/update/common/errors.h
@@ -79,16 +79,17 @@
 #define WRITE_ERROR_APPLY_DIR_PATH 65
 #define WRITE_ERROR_CALLBACK_PATH 66
 #define WRITE_ERROR_FILE_ACCESS_DENIED 67
 #define WRITE_ERROR_DIR_ACCESS_DENIED 68
 #define WRITE_ERROR_DELETE_BACKUP 69
 #define WRITE_ERROR_EXTRACT 70
 #define REMOVE_FILE_SPEC_ERROR 71
 #define INVALID_STAGED_PARENT_ERROR 72
+#define LOCK_ERROR_PATCH_FILE 73
 
 // Error codes 80 through 99 are reserved for nsUpdateService.js
 
 // The following error codes are only used by updater.exe
 // when a fallback key exists for tests.
 #define FALLBACKKEY_UNKNOWN_ERROR 100
 #define FALLBACKKEY_REGPATH_ERROR 101
 #define FALLBACKKEY_NOKEY_ERROR 102
--- a/toolkit/mozapps/update/updater/updater.cpp
+++ b/toolkit/mozapps/update/updater/updater.cpp
@@ -1372,28 +1372,41 @@ private:
   static int sPatchIndex;
 
   const NS_tchar *mPatchFile;
   const NS_tchar *mFile;
   int mPatchIndex;
   MBSPatchHeader header;
   unsigned char *buf;
   NS_tchar spath[MAXPATHLEN];
+  AutoFile mPatchStream;
 };
 
 int PatchFile::sPatchIndex = 0;
 
 PatchFile::~PatchFile()
 {
+  // Make sure mPatchStream gets unlocked on Windows; the system will do that,
+  // but not until some indeterminate future time, and we want determinism.
+  // Normally this happens at the end of Execute, when we close the stream;
+  // this call is here in case Execute errors out.
+#ifdef XP_WIN
+  if (mPatchStream) {
+    UnlockFile((HANDLE)_get_osfhandle(fileno(mPatchStream)), 0, 0, -1, -1);
+  }
+#endif
+
   // delete the temporary patch file
-  if (spath[0])
+  if (spath[0]) {
     NS_tremove(spath);
-
-  if (buf)
+  }
+
+  if (buf) {
     free(buf);
+  }
 }
 
 int
 PatchFile::LoadSourceFile(FILE* ofile)
 {
   struct stat os;
   int rv = fstat(fileno((FILE *)ofile), &os);
   if (rv) {
@@ -1404,18 +1417,19 @@ PatchFile::LoadSourceFile(FILE* ofile)
 
   if (uint32_t(os.st_size) != header.slen) {
     LOG(("LoadSourceFile: destination file size %d does not match expected size %d",
          uint32_t(os.st_size), header.slen));
     return LOADSOURCE_ERROR_WRONG_SIZE;
   }
 
   buf = (unsigned char *) malloc(header.slen);
-  if (!buf)
+  if (!buf) {
     return UPDATER_MEM_ERROR;
+  }
 
   size_t r = header.slen;
   unsigned char *rb = buf;
   while (r) {
     const size_t count = mmin(SSIZE_MAX, r);
     size_t c = fread(rb, 1, count, ofile);
     if (c != count) {
       LOG(("LoadSourceFile: error reading destination file: " LOG_S,
@@ -1442,27 +1456,30 @@ PatchFile::LoadSourceFile(FILE* ofile)
 
 int
 PatchFile::Parse(NS_tchar *line)
 {
   // format "<patchfile>" "<filetopatch>"
 
   // Get the path to the patch file inside of the mar
   mPatchFile = mstrtok(kQuote, &line);
-  if (!mPatchFile)
+  if (!mPatchFile) {
     return PARSE_ERROR;
+  }
 
   // consume whitespace between args
   NS_tchar *q = mstrtok(kQuote, &line);
-  if (!q)
+  if (!q) {
     return PARSE_ERROR;
+  }
 
   mFile = get_valid_path(&line);
-  if (!mFile)
+  if (!mFile) {
     return PARSE_ERROR;
+  }
 
   return OK;
 }
 
 int
 PatchFile::Prepare()
 {
   LOG(("PREPARE PATCH " LOG_S, mFile));
@@ -1470,48 +1487,55 @@ PatchFile::Prepare()
   // extract the patch to a temporary file
   mPatchIndex = sPatchIndex++;
 
   NS_tsnprintf(spath, sizeof(spath)/sizeof(spath[0]),
                NS_T("%s/updating/%d.patch"), gWorkingDirPath, mPatchIndex);
 
   NS_tremove(spath);
 
-  FILE *fp = NS_tfopen(spath, NS_T("wb"));
-  if (!fp)
+  mPatchStream = NS_tfopen(spath, NS_T("wb+"));
+  if (!mPatchStream) {
     return WRITE_ERROR;
+  }
 
 #ifdef XP_WIN
+  // Lock the patch file, so it can't be messed with between
+  // when we're done creating it and when we go to apply it.
+  if (!LockFile((HANDLE)_get_osfhandle(fileno(mPatchStream)), 0, 0, -1, -1)) {
+    LOG(("Couldn't lock patch file: %d", GetLastError()));
+    return LOCK_ERROR_PATCH_FILE;
+  }
+
   char sourcefile[MAXPATHLEN];
   if (!WideCharToMultiByte(CP_UTF8, 0, mPatchFile, -1, sourcefile, MAXPATHLEN,
                            nullptr, nullptr)) {
     LOG(("error converting wchar to utf8: %d", GetLastError()));
     return STRING_CONVERSION_ERROR;
   }
 
-  int rv = gArchiveReader.ExtractFileToStream(sourcefile, fp);
+  int rv = gArchiveReader.ExtractFileToStream(sourcefile, mPatchStream);
 #else
-  int rv = gArchiveReader.ExtractFileToStream(mPatchFile, fp);
+  int rv = gArchiveReader.ExtractFileToStream(mPatchFile, mPatchStream);
 #endif
-  fclose(fp);
+
   return rv;
 }
 
 int
 PatchFile::Execute()
 {
   LOG(("EXECUTE PATCH " LOG_S, mFile));
 
-  AutoFile pfile(NS_tfopen(spath, NS_T("rb")));
-  if (pfile == nullptr)
-    return READ_ERROR;
-
-  int rv = MBS_ReadHeader(pfile, &header);
-  if (rv)
+  fseek(mPatchStream, 0, SEEK_SET);
+
+  int rv = MBS_ReadHeader(mPatchStream, &header);
+  if (rv) {
     return rv;
+  }
 
   FILE *origfile = nullptr;
 #ifdef XP_WIN
   if (NS_tstrcmp(mFile, gCallbackRelPath) == 0) {
     // Read from the copy of the callback when patching since the callback can't
     // be opened for reading to prevent the application from being launched.
     origfile = NS_tfopen(gCallbackBackupPath, NS_T("rb"));
   } else {
@@ -1540,18 +1564,19 @@ PatchFile::Execute()
   rv = NS_tstat(mFile, &ss);
   if (rv) {
     LOG(("failed to read file status info: " LOG_S ", err: %d", mFile,
          errno));
     return READ_ERROR;
   }
 
   rv = backup_create(mFile);
-  if (rv)
+  if (rv) {
     return rv;
+  }
 
 #if defined(HAVE_POSIX_FALLOCATE)
   AutoFile ofile(ensure_open(mFile, NS_T("wb+"), ss.st_mode));
   posix_fallocate(fileno((FILE *)ofile), 0, header.dlen);
 #elif defined(XP_WIN)
   bool shouldTruncate = true;
   // Creating the file, setting the size, and then closing the file handle
   // lessens fragmentation more than any other method tested. Other methods that
@@ -1604,22 +1629,27 @@ PatchFile::Execute()
   }
 
 #ifdef XP_WIN
   if (!shouldTruncate) {
     fseek(ofile, 0, SEEK_SET);
   }
 #endif
 
-  rv = MBS_ApplyPatch(&header, pfile, buf, ofile);
+  rv = MBS_ApplyPatch(&header, mPatchStream, buf, ofile);
 
   // Go ahead and do a bit of cleanup now to minimize runtime overhead.
-  // Set pfile to nullptr to make AutoFile close the file so it can be deleted
-  // on Windows.
-  pfile = nullptr;
+  // Make sure mPatchStream gets unlocked on Windows; the system will do that,
+  // but not until some indeterminate future time, and we want determinism.
+#ifdef XP_WIN
+  UnlockFile((HANDLE)_get_osfhandle(fileno(mPatchStream)), 0, 0, -1, -1);
+#endif
+  // Set mPatchStream to nullptr to make AutoFile close the file,
+  // so it can be deleted on Windows.
+  mPatchStream = nullptr;
   NS_tremove(spath);
   spath[0] = NS_T('\0');
   free(buf);
   buf = nullptr;
 
   return rv;
 }