Merge "Fix potential race condition."
diff --git a/fs_mgr/Android.bp b/fs_mgr/Android.bp
index ffde114..3d3503c 100644
--- a/fs_mgr/Android.bp
+++ b/fs_mgr/Android.bp
@@ -35,6 +35,7 @@
     export_include_dirs: ["include"],
     include_dirs: ["system/vold"],
     srcs: [
+        "file_wait.cpp",
         "fs_mgr.cpp",
         "fs_mgr_format.cpp",
         "fs_mgr_verity.cpp",
diff --git a/fs_mgr/file_wait.cpp b/fs_mgr/file_wait.cpp
new file mode 100644
index 0000000..cbf6845
--- /dev/null
+++ b/fs_mgr/file_wait.cpp
@@ -0,0 +1,235 @@
+// Copyright (C) 2019 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include <fs_mgr/file_wait.h>
+
+#include <limits.h>
+#if defined(__linux__)
+#include <poll.h>
+#include <sys/inotify.h>
+#endif
+#if defined(WIN32)
+#include <io.h>
+#else
+#include <unistd.h>
+#endif
+
+#include <functional>
+#include <thread>
+
+#include <android-base/file.h>
+#include <android-base/logging.h>
+#include <android-base/unique_fd.h>
+
+namespace android {
+namespace fs_mgr {
+
+using namespace std::literals;
+using android::base::unique_fd;
+
+bool PollForFile(const std::string& path, const std::chrono::milliseconds relative_timeout) {
+    auto start_time = std::chrono::steady_clock::now();
+
+    while (true) {
+        if (!access(path.c_str(), F_OK) || errno != ENOENT) return true;
+
+        std::this_thread::sleep_for(50ms);
+
+        auto now = std::chrono::steady_clock::now();
+        auto time_elapsed = std::chrono::duration_cast<std::chrono::milliseconds>(now - start_time);
+        if (time_elapsed > relative_timeout) return false;
+    }
+}
+
+bool PollForFileDeleted(const std::string& path, const std::chrono::milliseconds relative_timeout) {
+    auto start_time = std::chrono::steady_clock::now();
+
+    while (true) {
+        if (access(path.c_str(), F_OK) && errno == ENOENT) return true;
+
+        std::this_thread::sleep_for(50ms);
+
+        auto now = std::chrono::steady_clock::now();
+        auto time_elapsed = std::chrono::duration_cast<std::chrono::milliseconds>(now - start_time);
+        if (time_elapsed > relative_timeout) return false;
+    }
+}
+
+#if defined(__linux__)
+class OneShotInotify {
+  public:
+    OneShotInotify(const std::string& path, uint32_t mask,
+                   const std::chrono::milliseconds relative_timeout);
+
+    bool Wait();
+
+  private:
+    bool CheckCompleted();
+    int64_t RemainingMs() const;
+    bool ConsumeEvents();
+
+    enum class Result { Success, Timeout, Error };
+    Result WaitImpl();
+
+    unique_fd inotify_fd_;
+    std::string path_;
+    uint32_t mask_;
+    std::chrono::time_point<std::chrono::steady_clock> start_time_;
+    std::chrono::milliseconds relative_timeout_;
+    bool finished_;
+};
+
+OneShotInotify::OneShotInotify(const std::string& path, uint32_t mask,
+                               const std::chrono::milliseconds relative_timeout)
+    : path_(path),
+      mask_(mask),
+      start_time_(std::chrono::steady_clock::now()),
+      relative_timeout_(relative_timeout),
+      finished_(false) {
+    // If the condition is already met, don't bother creating an inotify.
+    if (CheckCompleted()) return;
+
+    unique_fd inotify_fd(inotify_init1(IN_CLOEXEC | IN_NONBLOCK));
+    if (inotify_fd < 0) {
+        PLOG(ERROR) << "inotify_init1 failed";
+        return;
+    }
+
+    std::string watch_path;
+    if (mask == IN_CREATE) {
+        watch_path = android::base::Dirname(path);
+    } else {
+        watch_path = path;
+    }
+    if (inotify_add_watch(inotify_fd, watch_path.c_str(), mask) < 0) {
+        PLOG(ERROR) << "inotify_add_watch failed";
+        return;
+    }
+
+    // It's possible the condition was met before the add_watch. Check for
+    // this and abort early if so.
+    if (CheckCompleted()) return;
+
+    inotify_fd_ = std::move(inotify_fd);
+}
+
+bool OneShotInotify::Wait() {
+    Result result = WaitImpl();
+    if (result == Result::Success) return true;
+    if (result == Result::Timeout) return false;
+
+    // Some kind of error with inotify occurred, so fallback to a poll.
+    std::chrono::milliseconds timeout(RemainingMs());
+    if (mask_ == IN_CREATE) {
+        return PollForFile(path_, timeout);
+    } else if (mask_ == IN_DELETE_SELF) {
+        return PollForFileDeleted(path_, timeout);
+    } else {
+        LOG(ERROR) << "Unknown inotify mask: " << mask_;
+        return false;
+    }
+}
+
+OneShotInotify::Result OneShotInotify::WaitImpl() {
+    // If the operation completed super early, we'll never have created an
+    // inotify instance.
+    if (finished_) return Result::Success;
+    if (inotify_fd_ < 0) return Result::Error;
+
+    while (true) {
+        auto remaining_ms = RemainingMs();
+        if (remaining_ms <= 0) return Result::Timeout;
+
+        struct pollfd event = {
+                .fd = inotify_fd_,
+                .events = POLLIN,
+                .revents = 0,
+        };
+        int rv = poll(&event, 1, static_cast<int>(remaining_ms));
+        if (rv <= 0) {
+            if (rv == 0 || errno == EINTR) {
+                continue;
+            }
+            PLOG(ERROR) << "poll for inotify failed";
+            return Result::Error;
+        }
+        if (event.revents & POLLERR) {
+            LOG(ERROR) << "error reading inotify for " << path_;
+            return Result::Error;
+        }
+
+        // Note that we don't bother checking what kind of event it is, since
+        // it's cheap enough to just see if the initial condition is satisified.
+        // If it's not, we consume all the events available and continue.
+        if (CheckCompleted()) return Result::Success;
+        if (!ConsumeEvents()) return Result::Error;
+    }
+}
+
+bool OneShotInotify::CheckCompleted() {
+    if (mask_ == IN_CREATE) {
+        finished_ = !access(path_.c_str(), F_OK) || errno != ENOENT;
+    } else if (mask_ == IN_DELETE_SELF) {
+        finished_ = access(path_.c_str(), F_OK) && errno == ENOENT;
+    } else {
+        LOG(ERROR) << "Unexpected mask: " << mask_;
+    }
+    return finished_;
+}
+
+bool OneShotInotify::ConsumeEvents() {
+    // According to the manpage, this is enough to read at least one event.
+    static constexpr size_t kBufferSize = sizeof(struct inotify_event) + NAME_MAX + 1;
+    char buffer[kBufferSize];
+
+    do {
+        ssize_t rv = TEMP_FAILURE_RETRY(read(inotify_fd_, buffer, sizeof(buffer)));
+        if (rv <= 0) {
+            if (rv == 0 || errno == EAGAIN) {
+                return true;
+            }
+            PLOG(ERROR) << "read inotify failed";
+            return false;
+        }
+    } while (true);
+}
+
+int64_t OneShotInotify::RemainingMs() const {
+    auto remaining = (std::chrono::steady_clock::now() - start_time_);
+    auto elapsed = std::chrono::duration_cast<std::chrono::milliseconds>(remaining);
+    return (relative_timeout_ - elapsed).count();
+}
+#endif
+
+bool WaitForFile(const std::string& path, const std::chrono::milliseconds relative_timeout) {
+#if defined(__linux__)
+    OneShotInotify inotify(path, IN_CREATE, relative_timeout);
+    return inotify.Wait();
+#else
+    return PollForFile(path, relative_timeout);
+#endif
+}
+
+// Wait at most |relative_timeout| milliseconds for |path| to stop existing.
+bool WaitForFileDeleted(const std::string& path, const std::chrono::milliseconds relative_timeout) {
+#if defined(__linux__)
+    OneShotInotify inotify(path, IN_DELETE_SELF, relative_timeout);
+    return inotify.Wait();
+#else
+    return PollForFileDeleted(path, relative_timeout);
+#endif
+}
+
+}  // namespace fs_mgr
+}  // namespace android
diff --git a/fs_mgr/include/fs_mgr/file_wait.h b/fs_mgr/include/fs_mgr/file_wait.h
new file mode 100644
index 0000000..74d160e
--- /dev/null
+++ b/fs_mgr/include/fs_mgr/file_wait.h
@@ -0,0 +1,34 @@
+// Copyright (C) 2019 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#pragma once
+
+#include <chrono>
+#include <string>
+
+namespace android {
+namespace fs_mgr {
+
+// Wait at most |relative_timeout| milliseconds for |path| to exist. dirname(path)
+// must already exist. For example, to wait on /dev/block/dm-6, /dev/block must
+// be a valid directory.
+bool WaitForFile(const std::string& path, const std::chrono::milliseconds relative_timeout);
+
+// Wait at most |relative_timeout| milliseconds for |path| to stop existing.
+// Note that this only returns true if the inode itself no longer exists, i.e.,
+// all outstanding file descriptors have been closed.
+bool WaitForFileDeleted(const std::string& path, const std::chrono::milliseconds relative_timeout);
+
+}  // namespace fs_mgr
+}  // namespace android
diff --git a/fs_mgr/tests/Android.bp b/fs_mgr/tests/Android.bp
index eb9f525..83668e9 100644
--- a/fs_mgr/tests/Android.bp
+++ b/fs_mgr/tests/Android.bp
@@ -25,6 +25,7 @@
         "libfstab",
     ],
     srcs: [
+        "file_wait_test.cpp",
         "fs_mgr_test.cpp",
     ],
 
diff --git a/fs_mgr/tests/file_wait_test.cpp b/fs_mgr/tests/file_wait_test.cpp
new file mode 100644
index 0000000..cc8b143
--- /dev/null
+++ b/fs_mgr/tests/file_wait_test.cpp
@@ -0,0 +1,91 @@
+// Copyright (C) 2019 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include <chrono>
+#include <string>
+#include <thread>
+
+#include <android-base/file.h>
+#include <android-base/unique_fd.h>
+#include <fs_mgr/file_wait.h>
+#include <gtest/gtest.h>
+
+using namespace std::literals;
+using android::base::unique_fd;
+using android::fs_mgr::WaitForFile;
+using android::fs_mgr::WaitForFileDeleted;
+
+class FileWaitTest : public ::testing::Test {
+  protected:
+    void SetUp() override {
+        const ::testing::TestInfo* tinfo = ::testing::UnitTest::GetInstance()->current_test_info();
+        test_file_ = temp_dir_.path + "/"s + tinfo->name();
+    }
+
+    void TearDown() override { unlink(test_file_.c_str()); }
+
+    TemporaryDir temp_dir_;
+    std::string test_file_;
+};
+
+TEST_F(FileWaitTest, FileExists) {
+    unique_fd fd(open(test_file_.c_str(), O_CREAT | O_TRUNC | O_RDWR, 0700));
+    ASSERT_GE(fd, 0);
+
+    ASSERT_TRUE(WaitForFile(test_file_, 500ms));
+    ASSERT_FALSE(WaitForFileDeleted(test_file_, 500ms));
+}
+
+TEST_F(FileWaitTest, FileDoesNotExist) {
+    ASSERT_FALSE(WaitForFile(test_file_, 500ms));
+    ASSERT_TRUE(WaitForFileDeleted(test_file_, 500ms));
+}
+
+TEST_F(FileWaitTest, CreateAsync) {
+    std::thread thread([this] {
+        std::this_thread::sleep_for(std::chrono::seconds(1));
+        unique_fd fd(open(test_file_.c_str(), O_CREAT | O_TRUNC | O_RDWR, 0700));
+    });
+    EXPECT_TRUE(WaitForFile(test_file_, 3s));
+    thread.join();
+}
+
+TEST_F(FileWaitTest, CreateOtherAsync) {
+    std::thread thread([this] {
+        std::this_thread::sleep_for(std::chrono::seconds(1));
+        unique_fd fd(open(test_file_.c_str(), O_CREAT | O_TRUNC | O_RDWR, 0700));
+    });
+    EXPECT_FALSE(WaitForFile(test_file_ + ".wontexist", 2s));
+    thread.join();
+}
+
+TEST_F(FileWaitTest, DeleteAsync) {
+    // Note: need to close the file, otherwise inotify considers it not deleted.
+    {
+        unique_fd fd(open(test_file_.c_str(), O_CREAT | O_TRUNC | O_RDWR, 0700));
+        ASSERT_GE(fd, 0);
+    }
+
+    std::thread thread([this] {
+        std::this_thread::sleep_for(std::chrono::seconds(1));
+        unlink(test_file_.c_str());
+    });
+    EXPECT_TRUE(WaitForFileDeleted(test_file_, 3s));
+    thread.join();
+}
+
+TEST_F(FileWaitTest, BadPath) {
+    ASSERT_FALSE(WaitForFile("/this/path/does/not/exist", 5ms));
+    EXPECT_EQ(errno, ENOENT);
+}
diff --git a/libziparchive/include/ziparchive/zip_writer.h b/libziparchive/include/ziparchive/zip_writer.h
index bd44fdb..a2a0dbf 100644
--- a/libziparchive/include/ziparchive/zip_writer.h
+++ b/libziparchive/include/ziparchive/zip_writer.h
@@ -21,6 +21,7 @@
 
 #include <memory>
 #include <string>
+#include <string_view>
 #include <vector>
 
 #include "android-base/macros.h"
@@ -101,7 +102,7 @@
    * Subsequent calls to WriteBytes(const void*, size_t) will add data to this entry.
    * Returns 0 on success, and an error value < 0 on failure.
    */
-  int32_t StartEntry(const char* path, size_t flags);
+  int32_t StartEntry(std::string_view path, size_t flags);
 
   /**
    * Starts a new zip entry with the given path and flags, where the
@@ -111,17 +112,17 @@
    * Subsequent calls to WriteBytes(const void*, size_t) will add data to this entry.
    * Returns 0 on success, and an error value < 0 on failure.
    */
-  int32_t StartAlignedEntry(const char* path, size_t flags, uint32_t alignment);
+  int32_t StartAlignedEntry(std::string_view path, size_t flags, uint32_t alignment);
 
   /**
    * Same as StartEntry(const char*, size_t), but sets a last modified time for the entry.
    */
-  int32_t StartEntryWithTime(const char* path, size_t flags, time_t time);
+  int32_t StartEntryWithTime(std::string_view path, size_t flags, time_t time);
 
   /**
    * Same as StartAlignedEntry(const char*, size_t), but sets a last modified time for the entry.
    */
-  int32_t StartAlignedEntryWithTime(const char* path, size_t flags, time_t time, uint32_t alignment);
+  int32_t StartAlignedEntryWithTime(std::string_view path, size_t flags, time_t time, uint32_t alignment);
 
   /**
    * Writes bytes to the zip file for the previously started zip entry.
diff --git a/libziparchive/zip_archive_benchmark.cpp b/libziparchive/zip_archive_benchmark.cpp
index 23ed408..09d3b8a 100644
--- a/libziparchive/zip_archive_benchmark.cpp
+++ b/libziparchive/zip_archive_benchmark.cpp
@@ -58,7 +58,7 @@
   std::string_view name("thisFileNameDoesNotExist");
 
   // Start the benchmark.
-  while (state.KeepRunning()) {
+  for (auto _ : state) {
     OpenArchive(temp_file->path, &handle);
     FindEntry(handle, name, &data);
     CloseArchive(handle);
@@ -73,7 +73,7 @@
   ZipEntry data;
   std::string name;
 
-  while (state.KeepRunning()) {
+  for (auto _ : state) {
     OpenArchive(temp_file->path, &handle);
     StartIteration(handle, &iteration_cookie);
     while (Next(iteration_cookie, &data, &name) == 0) {
@@ -84,4 +84,27 @@
 }
 BENCHMARK(Iterate_all_files);
 
+static void StartAlignedEntry(benchmark::State& state) {
+  TemporaryFile file;
+  FILE* fp = fdopen(file.fd, "w");
+
+  ZipWriter writer(fp);
+
+  auto alignment = uint32_t(state.range(0));
+  std::string name = "name";
+  int counter = 0;
+  for (auto _ : state) {
+    writer.StartAlignedEntry(name + std::to_string(counter++), 0, alignment);
+    state.PauseTiming();
+    writer.WriteBytes("hola", 4);
+    writer.FinishEntry();
+    state.ResumeTiming();
+  }
+
+  writer.Finish();
+  fclose(fp);
+}
+BENCHMARK(StartAlignedEntry)->Arg(2)->Arg(16)->Arg(1024)->Arg(4096);
+
+
 BENCHMARK_MAIN();
diff --git a/libziparchive/zip_writer.cc b/libziparchive/zip_writer.cc
index ae9d145..198154b 100644
--- a/libziparchive/zip_writer.cc
+++ b/libziparchive/zip_writer.cc
@@ -130,7 +130,7 @@
   return error_code;
 }
 
-int32_t ZipWriter::StartEntry(const char* path, size_t flags) {
+int32_t ZipWriter::StartEntry(std::string_view path, size_t flags) {
   uint32_t alignment = 0;
   if (flags & kAlign32) {
     flags &= ~kAlign32;
@@ -139,11 +139,11 @@
   return StartAlignedEntryWithTime(path, flags, time_t(), alignment);
 }
 
-int32_t ZipWriter::StartAlignedEntry(const char* path, size_t flags, uint32_t alignment) {
+int32_t ZipWriter::StartAlignedEntry(std::string_view path, size_t flags, uint32_t alignment) {
   return StartAlignedEntryWithTime(path, flags, time_t(), alignment);
 }
 
-int32_t ZipWriter::StartEntryWithTime(const char* path, size_t flags, time_t time) {
+int32_t ZipWriter::StartEntryWithTime(std::string_view path, size_t flags, time_t time) {
   uint32_t alignment = 0;
   if (flags & kAlign32) {
     flags &= ~kAlign32;
@@ -198,7 +198,7 @@
   dst->extra_field_length = src.padding_length;
 }
 
-int32_t ZipWriter::StartAlignedEntryWithTime(const char* path, size_t flags, time_t time,
+int32_t ZipWriter::StartAlignedEntryWithTime(std::string_view path, size_t flags, time_t time,
                                              uint32_t alignment) {
   if (state_ != State::kWritingZip) {
     return kInvalidState;
@@ -247,13 +247,24 @@
   ExtractTimeAndDate(time, &file_entry.last_mod_time, &file_entry.last_mod_date);
 
   off_t offset = current_offset_ + sizeof(LocalFileHeader) + file_entry.path.size();
-  std::vector<char> zero_padding;
+  // prepare a pre-zeroed memory page in case when we need to pad some aligned data.
+  static constexpr auto kPageSize = 4096;
+  static constexpr char kSmallZeroPadding[kPageSize] = {};
+  // use this buffer if our preallocated one is too small
+  std::vector<char> zero_padding_big;
+  const char* zero_padding = nullptr;
+
   if (alignment != 0 && (offset & (alignment - 1))) {
     // Pad the extra field so the data will be aligned.
     uint16_t padding = static_cast<uint16_t>(alignment - (offset % alignment));
     file_entry.padding_length = padding;
     offset += padding;
-    zero_padding.resize(padding, 0);
+    if (padding <= std::size(kSmallZeroPadding)) {
+        zero_padding = kSmallZeroPadding;
+    } else {
+        zero_padding_big.resize(padding, 0);
+        zero_padding = zero_padding_big.data();
+    }
   }
 
   LocalFileHeader header = {};
@@ -265,11 +276,11 @@
     return HandleError(kIoError);
   }
 
-  if (fwrite(path, sizeof(*path), file_entry.path.size(), file_) != file_entry.path.size()) {
+  if (fwrite(path.data(), 1, path.size(), file_) != path.size()) {
     return HandleError(kIoError);
   }
 
-  if (file_entry.padding_length != 0 && fwrite(zero_padding.data(), 1, file_entry.padding_length,
+  if (file_entry.padding_length != 0 && fwrite(zero_padding, 1, file_entry.padding_length,
                                                file_) != file_entry.padding_length) {
     return HandleError(kIoError);
   }