From 3a8d98dd902da8b69cff731b3db03c75b176b751 Mon Sep 17 00:00:00 2001 From: Tianjie Xu Date: Mon, 3 Apr 2017 20:01:17 -0700 Subject: Abort the update if there's not enough new data Right now the update stuck in a deadlock if there's less new data than expection. Add some checkers and abort the update if such case happens. Also add a corresponding test. Bug: 36787146 Test: update aborts correctly on bullhead && recovery_component_test passes Change-Id: I914e4a2a4cf157b99ef2fc65bd21c6981e38ca47 --- tests/component/updater_test.cpp | 66 ++++++++++++++++++++++++++++++++++++++++ updater/blockimg.cpp | 30 +++++++++++++++--- 2 files changed, 92 insertions(+), 4 deletions(-) diff --git a/tests/component/updater_test.cpp b/tests/component/updater_test.cpp index 5652ddf46..dc4b5d724 100644 --- a/tests/component/updater_test.cpp +++ b/tests/component/updater_test.cpp @@ -607,3 +607,69 @@ TEST_F(UpdaterTest, block_image_update) { 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 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); +} diff --git a/updater/blockimg.cpp b/updater/blockimg.cpp index 0f08d17eb..d5c170473 100644 --- a/updater/blockimg.cpp +++ b/updater/blockimg.cpp @@ -149,7 +149,7 @@ static void allocate(size_t size, std::vector& buffer) { 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(0)); }; @@ -201,9 +201,14 @@ class RangeSinkWriter { 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 @@ class RangeSinkWriter { 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 @@ struct NewThreadInfo { ZipEntry entry; RangeSinkWriter* writer; + bool receiver_available; pthread_mutex_t mu; pthread_cond_t cv; @@ -274,9 +282,16 @@ static bool receive_new_data(const uint8_t* data, size_t size, void* cookie) { } static void* unzip_new_data(void* cookie) { - NewThreadInfo* nti = static_cast(cookie); - ProcessZipEntryContents(nti->za, &nti->entry, receive_new_data, nti); - return nullptr; + NewThreadInfo* nti = static_cast(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& buffer, int fd) { @@ -1133,6 +1148,12 @@ static int PerformCommandNew(CommandParameters& params) { pthread_cond_broadcast(¶ms.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(¶ms.nti.mu); + return -1; + } pthread_cond_wait(¶ms.nti.cv, ¶ms.nti.mu); } @@ -1361,6 +1382,7 @@ static Value* PerformBlockImageUpdate(const char* name, State* state, if (params.canwrite) { params.nti.za = za; params.nti.entry = new_entry; + params.nti.receiver_available = true; pthread_mutex_init(¶ms.nti.mu, nullptr); pthread_cond_init(¶ms.nti.cv, nullptr); -- cgit v1.2.3