commit b4b0c49c48bc0d699942f8eec9e2e7110eefbe66
author Tianjie Xu <xunchang@google.com> Mon Apr 10 18:12:08 2017 +0000
committer Gerrit Code Review <noreply-gerritcodereview@google.com> Mon Apr 10 18:12:09 2017 +0000
tree aae8c6d44aea111c2c4813e4caae27cd2c4db5ce
parent 3cd630a5a8d5a19dda9429af5494f19625bc2fa0
parent 3a8d98dd902da8b69cff731b3db03c75b176b751

Merge "Abort the update if there's not enough new data"

tests/component/updater_test.cpp

diff --git a/tests/component/updater_test.cpp b/tests/component/updater_test.cpp
index 5652ddf..dc4b5d7 100644
--- a/tests/component/updater_test.cpp
+++ b/tests/component/updater_test.cpp
@@ -607,3 +607,69 @@
   ASSERT_EQ(0, fclose(updater_info.cmd_pipe));
   CloseArchive(handle);
 }
+
+TEST_F(UpdaterTest, new_data_short_write) {
+  // Create a zip file with new_data.
+  TemporaryFile zip_file;
+  FILE* zip_file_ptr = fdopen(zip_file.fd, "wb");
+  ZipWriter zip_writer(zip_file_ptr);
+
+  // Add the empty new data.
+  ASSERT_EQ(0, zip_writer.StartEntry("empty_new_data", 0));
+  ASSERT_EQ(0, zip_writer.FinishEntry());
+  // Add the short written new data.
+  ASSERT_EQ(0, zip_writer.StartEntry("short_new_data", 0));
+  std::string new_data_short = std::string(10, 'a');
+  ASSERT_EQ(0, zip_writer.WriteBytes(new_data_short.data(), new_data_short.size()));
+  ASSERT_EQ(0, zip_writer.FinishEntry());
+  // Add the data of exactly one block.
+  ASSERT_EQ(0, zip_writer.StartEntry("exact_new_data", 0));
+  std::string new_data_exact = std::string(4096, 'a');
+  ASSERT_EQ(0, zip_writer.WriteBytes(new_data_exact.data(), new_data_exact.size()));
+  ASSERT_EQ(0, zip_writer.FinishEntry());
+  // Add a dummy patch data.
+  ASSERT_EQ(0, zip_writer.StartEntry("patch_data", 0));
+  ASSERT_EQ(0, zip_writer.FinishEntry());
+
+  std::vector<std::string> transfer_list = {
+    "4",
+    "1",
+    "0",
+    "0",
+    "new 2,0,1",
+  };
+  ASSERT_EQ(0, zip_writer.StartEntry("transfer_list", 0));
+  std::string commands = android::base::Join(transfer_list, '\n');
+  ASSERT_EQ(0, zip_writer.WriteBytes(commands.data(), commands.size()));
+  ASSERT_EQ(0, zip_writer.FinishEntry());
+  ASSERT_EQ(0, zip_writer.Finish());
+  ASSERT_EQ(0, fclose(zip_file_ptr));
+
+  MemMapping map;
+  ASSERT_EQ(0, sysMapFile(zip_file.path, &map));
+  ZipArchiveHandle handle;
+  ASSERT_EQ(0, OpenArchiveFromMemory(map.addr, map.length, zip_file.path, &handle));
+
+  // Set up the handler, command_pipe, patch offset & length.
+  UpdaterInfo updater_info;
+  updater_info.package_zip = handle;
+  TemporaryFile temp_pipe;
+  updater_info.cmd_pipe = fopen(temp_pipe.path, "wb");
+  updater_info.package_zip_addr = map.addr;
+  updater_info.package_zip_len = map.length;
+
+  // Updater should report the failure gracefully rather than stuck in deadlock.
+  TemporaryFile update_file;
+  std::string script_empty_data = "block_image_update(\"" + std::string(update_file.path) +
+      R"(", package_extract_file("transfer_list"), "empty_new_data", "patch_data"))";
+  expect("", script_empty_data.c_str(), kNoCause, &updater_info);
+
+  std::string script_short_data = "block_image_update(\"" + std::string(update_file.path) +
+      R"(", package_extract_file("transfer_list"), "short_new_data", "patch_data"))";
+  expect("", script_short_data.c_str(), kNoCause, &updater_info);
+
+  // Expect to write 1 block of new data successfully.
+  std::string script_exact_data = "block_image_update(\"" + std::string(update_file.path) +
+      R"(", package_extract_file("transfer_list"), "exact_new_data", "patch_data"))";
+  expect("t", script_exact_data.c_str(), kNoCause, &updater_info);
+}

updater/blockimg.cpp

diff --git a/updater/blockimg.cpp b/updater/blockimg.cpp
index 0f08d17..d5c1704 100644
--- a/updater/blockimg.cpp
+++ b/updater/blockimg.cpp
@@ -149,7 +149,7 @@
 class RangeSinkWriter {
  public:
   RangeSinkWriter(int fd, const RangeSet& tgt)
-      : fd_(fd), tgt_(tgt), next_range_(0), current_range_left_(0) {
+      : fd_(fd), tgt_(tgt), next_range_(0), current_range_left_(0), bytes_written_(0) {
     CHECK_NE(tgt.size(), static_cast<size_t>(0));
   };
 
@@ -201,9 +201,14 @@
       written += write_now;
     }
 
+    bytes_written_ += written;
     return written;
   }
 
+  size_t BytesWritten() const {
+    return bytes_written_;
+  }
+
  private:
   // The input data.
   int fd_;
@@ -213,6 +218,8 @@
   size_t next_range_;
   // The number of bytes to write before moving to the next range.
   size_t current_range_left_;
+  // Total bytes written by the writer.
+  size_t bytes_written_;
 };
 
 /**
@@ -238,6 +245,7 @@
   ZipEntry entry;
 
   RangeSinkWriter* writer;
+  bool receiver_available;
 
   pthread_mutex_t mu;
   pthread_cond_t cv;
@@ -274,9 +282,16 @@
 }
 
 static void* unzip_new_data(void* cookie) {
-    NewThreadInfo* nti = static_cast<NewThreadInfo*>(cookie);
-    ProcessZipEntryContents(nti->za, &nti->entry, receive_new_data, nti);
-    return nullptr;
+  NewThreadInfo* nti = static_cast<NewThreadInfo*>(cookie);
+  ProcessZipEntryContents(nti->za, &nti->entry, receive_new_data, nti);
+
+  pthread_mutex_lock(&nti->mu);
+  nti->receiver_available = false;
+  if (nti->writer != nullptr) {
+    pthread_cond_broadcast(&nti->cv);
+  }
+  pthread_mutex_unlock(&nti->mu);
+  return nullptr;
 }
 
 static int ReadBlocks(const RangeSet& src, std::vector<uint8_t>& buffer, int fd) {
@@ -1133,6 +1148,12 @@
     pthread_cond_broadcast(&params.nti.cv);
 
     while (params.nti.writer != nullptr) {
+      if (!params.nti.receiver_available) {
+        LOG(ERROR) << "missing " << (tgt.blocks() * BLOCKSIZE - params.nti.writer->BytesWritten())
+                   << " bytes of new data";
+        pthread_mutex_unlock(&params.nti.mu);
+        return -1;
+      }
       pthread_cond_wait(&params.nti.cv, &params.nti.mu);
     }
 
@@ -1361,6 +1382,7 @@
   if (params.canwrite) {
     params.nti.za = za;
     params.nti.entry = new_entry;
+    params.nti.receiver_available = true;
 
     pthread_mutex_init(&params.nti.mu, nullptr);
     pthread_cond_init(&params.nti.cv, nullptr);