Bug 1237219 - Lock patch files while applying updates; r=rstrong a=ritu
authorMatt Howell <mhowell@mozilla.com>
Thu, 14 Apr 2016 09:33:42 -0700
changeset 324440 fdb6793f1e4a6b6bb6851adc2c322f1e552e64f2
parent 324439 bb07b5227ff1683f14ae78cdd630164969335608
child 324441 b9973c3b3c5c0c577ab64da811e125f102de7de0
push id6014
push userkwierso@gmail.com
push dateFri, 20 May 2016 23:25:00 +0000
treeherdermozilla-beta@bcc547cc6eec [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersrstrong, ritu
bugs1237219
milestone47.0
Bug 1237219 - Lock patch files while applying updates; r=rstrong a=ritu 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;
 }