Merge "[adb] file sync performance on Windows" into rvc-dev
diff --git a/adb/client/file_sync_client.cpp b/adb/client/file_sync_client.cpp
index f29ca92..ed4a1fe 100644
--- a/adb/client/file_sync_client.cpp
+++ b/adb/client/file_sync_client.cpp
@@ -223,7 +223,8 @@
 
 class SyncConnection {
   public:
-    SyncConnection() {
+    SyncConnection() : acknowledgement_buffer_(sizeof(sync_status) + SYNC_DATA_MAX) {
+        acknowledgement_buffer_.resize(0);
         max = SYNC_DATA_MAX; // TODO: decide at runtime.
 
         std::string error;
@@ -521,34 +522,6 @@
         return WriteOrDie(lpath, rpath, &msg.data, sizeof(msg.data));
     }
 
-    bool ReadAcknowledgments() {
-        bool result = true;
-        while (!deferred_acknowledgements_.empty()) {
-            auto [from, to] = std::move(deferred_acknowledgements_.front());
-            deferred_acknowledgements_.pop_front();
-            result &= CopyDone(from, to);
-        }
-        return result;
-    }
-
-    bool CopyDone(const std::string& from, const std::string& to) {
-        syncmsg msg;
-        if (!ReadFdExactly(fd, &msg.status, sizeof(msg.status))) {
-            Error("failed to copy '%s' to '%s': couldn't read from device", from.c_str(),
-                  to.c_str());
-            return false;
-        }
-        if (msg.status.id == ID_OKAY) {
-            return true;
-        }
-        if (msg.status.id != ID_FAIL) {
-            Error("failed to copy '%s' to '%s': unknown reason %d", from.c_str(), to.c_str(),
-                  msg.status.id);
-            return false;
-        }
-        return ReportCopyFailure(from, to, msg);
-    }
-
     bool ReportCopyFailure(const std::string& from, const std::string& to, const syncmsg& msg) {
         std::vector<char> buf(msg.status.msglen + 1);
         if (!ReadFdExactly(fd, &buf[0], msg.status.msglen)) {
@@ -561,6 +534,97 @@
         return false;
     }
 
+    void CopyDone() { deferred_acknowledgements_.pop_front(); }
+
+    void ReportDeferredCopyFailure(const std::string& msg) {
+        auto& [from, to] = deferred_acknowledgements_.front();
+        Error("failed to copy '%s' to '%s': remote %s", from.c_str(), to.c_str(), msg.c_str());
+        deferred_acknowledgements_.pop_front();
+    }
+
+    bool ReadAcknowledgements(bool read_all = false) {
+        // We need to read enough such that adbd's intermediate socket's write buffer can't be
+        // full. The default buffer on Linux is 212992 bytes, but there's 576 bytes of bookkeeping
+        // overhead per write. The worst case scenario is a continuous string of failures, since
+        // each logical packet is divided into two writes. If our packet size if conservatively 512
+        // bytes long, this leaves us with space for 128 responses.
+        constexpr size_t max_deferred_acks = 128;
+        auto& buf = acknowledgement_buffer_;
+        adb_pollfd pfd = {.fd = fd.get(), .events = POLLIN};
+        while (!deferred_acknowledgements_.empty()) {
+            bool should_block = read_all || deferred_acknowledgements_.size() >= max_deferred_acks;
+
+            ssize_t rc = adb_poll(&pfd, 1, should_block ? -1 : 0);
+            if (rc == 0) {
+                CHECK(!should_block);
+                return true;
+            }
+
+            if (acknowledgement_buffer_.size() < sizeof(sync_status)) {
+                const ssize_t header_bytes_left = sizeof(sync_status) - buf.size();
+                ssize_t rc = adb_read(fd, buf.end(), header_bytes_left);
+                if (rc <= 0) {
+                    Error("failed to read copy response");
+                    return false;
+                }
+
+                buf.resize(buf.size() + rc);
+                if (rc != header_bytes_left) {
+                    // Early exit if we run out of data in the socket.
+                    return true;
+                }
+
+                if (!should_block) {
+                    // We don't want to read again yet, because the socket might be empty.
+                    continue;
+                }
+            }
+
+            auto* hdr = reinterpret_cast<sync_status*>(buf.data());
+            if (hdr->id == ID_OKAY) {
+                buf.resize(0);
+                if (hdr->msglen != 0) {
+                    Error("received ID_OKAY with msg_len (%" PRIu32 " != 0", hdr->msglen);
+                    return false;
+                }
+                CopyDone();
+                continue;
+            } else if (hdr->id != ID_FAIL) {
+                Error("unexpected response from daemon: id = %#" PRIx32, hdr->id);
+                return false;
+            } else if (hdr->msglen > SYNC_DATA_MAX) {
+                Error("too-long message length from daemon: msglen = %" PRIu32, hdr->msglen);
+                return false;
+            }
+
+            const ssize_t msg_bytes_left = hdr->msglen + sizeof(sync_status) - buf.size();
+            CHECK_GE(msg_bytes_left, 0);
+            if (msg_bytes_left > 0) {
+                ssize_t rc = adb_read(fd, buf.end(), msg_bytes_left);
+                if (rc <= 0) {
+                    Error("failed to read copy failure message");
+                    return false;
+                }
+
+                buf.resize(buf.size() + rc);
+                if (rc != msg_bytes_left) {
+                    if (should_block) {
+                        continue;
+                    } else {
+                        return true;
+                    }
+                }
+
+                std::string msg(buf.begin() + sizeof(sync_status), buf.end());
+                ReportDeferredCopyFailure(msg);
+                buf.resize(0);
+                return false;
+            }
+        }
+
+        return true;
+    }
+
     void Printf(const char* fmt, ...) __attribute__((__format__(__printf__, 2, 3))) {
         std::string s;
 
@@ -627,6 +691,7 @@
 
   private:
     std::deque<std::pair<std::string, std::string>> deferred_acknowledgements_;
+    Block acknowledgement_buffer_;
     FeatureSet features_;
     bool have_stat_v2_;
     bool have_ls_v2_;
@@ -735,7 +800,7 @@
         if (!sc.SendSmallFile(rpath, mode, lpath, rpath, mtime, buf, data_length)) {
             return false;
         }
-        return true;
+        return sc.ReadAcknowledgements();
 #endif
     }
 
@@ -758,7 +823,7 @@
             return false;
         }
     }
-    return true;
+    return sc.ReadAcknowledgements();
 }
 
 static bool sync_recv(SyncConnection& sc, const char* rpath, const char* lpath,
@@ -985,8 +1050,9 @@
     }
 
     sc.RecordFilesSkipped(skipped);
+    bool success = sc.ReadAcknowledgements(true);
     sc.ReportTransferRate(lpath, TransferDirection::push);
-    return true;
+    return success;
 }
 
 bool do_sync_push(const std::vector<const char*>& srcs, const char* dst, bool sync) {
@@ -1079,7 +1145,7 @@
         sc.ReportTransferRate(src_path, TransferDirection::push);
     }
 
-    success &= sc.ReadAcknowledgments();
+    success &= sc.ReadAcknowledgements(true);
     sc.ReportOverallTransferRate(TransferDirection::push);
     return success;
 }
diff --git a/adb/client/incremental_server.cpp b/adb/client/incremental_server.cpp
index 737563c..4a131ce 100644
--- a/adb/client/incremental_server.cpp
+++ b/adb/client/incremental_server.cpp
@@ -46,8 +46,9 @@
 
 static constexpr int kBlockSize = 4096;
 static constexpr int kCompressedSizeMax = kBlockSize * 0.95;
-static constexpr short kCompressionNone = 0;
-static constexpr short kCompressionLZ4 = 1;
+static constexpr int8_t kTypeData = 0;
+static constexpr int8_t kCompressionNone = 0;
+static constexpr int8_t kCompressionLZ4 = 1;
 static constexpr int kCompressBound = std::max(kBlockSize, LZ4_COMPRESSBOUND(kBlockSize));
 static constexpr auto kReadBufferSize = 128 * 1024;
 static constexpr int kPollTimeoutMillis = 300000;  // 5 minutes
@@ -56,7 +57,8 @@
 using FileId = int16_t;
 using BlockIdx = int32_t;
 using NumBlocks = int32_t;
-using CompressionType = int16_t;
+using BlockType = int8_t;
+using CompressionType = int8_t;
 using RequestType = int16_t;
 using ChunkHeader = int32_t;
 using MagicType = uint32_t;
@@ -126,7 +128,8 @@
 // Placed before actual data bytes of each block
 struct ResponseHeader {
     FileId file_id;                    // 2 bytes
-    CompressionType compression_type;  // 2 bytes
+    BlockType block_type;              // 1 byte
+    CompressionType compression_type;  // 1 byte
     BlockIdx block_idx;                // 4 bytes
     BlockSize block_size;              // 2 bytes
 } __attribute__((packed));
@@ -343,14 +346,16 @@
         ++compressed_;
         blockSize = compressedSize;
         header = reinterpret_cast<ResponseHeader*>(data);
-        header->compression_type = toBigEndian(kCompressionLZ4);
+        header->compression_type = kCompressionLZ4;
     } else {
         ++uncompressed_;
         blockSize = bytesRead;
         header = reinterpret_cast<ResponseHeader*>(raw);
-        header->compression_type = toBigEndian(kCompressionNone);
+        header->compression_type = kCompressionNone;
     }
 
+    header->block_type = kTypeData;
+
     header->file_id = toBigEndian(fileId);
     header->block_size = toBigEndian(blockSize);
     header->block_idx = toBigEndian(blockIdx);
@@ -364,6 +369,7 @@
 bool IncrementalServer::SendDone() {
     ResponseHeader header;
     header.file_id = -1;
+    header.block_type = 0;
     header.compression_type = 0;
     header.block_idx = 0;
     header.block_size = 0;
diff --git a/adb/file_sync_protocol.h b/adb/file_sync_protocol.h
index 508c138..87ede0c 100644
--- a/adb/file_sync_protocol.h
+++ b/adb/file_sync_protocol.h
@@ -41,57 +41,69 @@
     // Followed by 'path_length' bytes of path (not NUL-terminated).
 } __attribute__((packed));
 
+struct __attribute__((packed)) sync_stat_v1 {
+    uint32_t id;
+    uint32_t mode;
+    uint32_t size;
+    uint32_t mtime;
+};
+
+struct __attribute__((packed)) sync_stat_v2 {
+    uint32_t id;
+    uint32_t error;
+    uint64_t dev;
+    uint64_t ino;
+    uint32_t mode;
+    uint32_t nlink;
+    uint32_t uid;
+    uint32_t gid;
+    uint64_t size;
+    int64_t atime;
+    int64_t mtime;
+    int64_t ctime;
+};
+
+struct __attribute__((packed)) sync_dent_v1 {
+    uint32_t id;
+    uint32_t mode;
+    uint32_t size;
+    uint32_t mtime;
+    uint32_t namelen;
+};  // followed by `namelen` bytes of the name.
+
+struct __attribute__((packed)) sync_dent_v2 {
+    uint32_t id;
+    uint32_t error;
+    uint64_t dev;
+    uint64_t ino;
+    uint32_t mode;
+    uint32_t nlink;
+    uint32_t uid;
+    uint32_t gid;
+    uint64_t size;
+    int64_t atime;
+    int64_t mtime;
+    int64_t ctime;
+    uint32_t namelen;
+};  // followed by `namelen` bytes of the name.
+
+struct __attribute__((packed)) sync_data {
+    uint32_t id;
+    uint32_t size;
+};  // followed by `size` bytes of data.
+
+struct __attribute__((packed)) sync_status {
+    uint32_t id;
+    uint32_t msglen;
+};  // followed by `msglen` bytes of error message, if id == ID_FAIL.
+
 union syncmsg {
-    struct __attribute__((packed)) {
-        uint32_t id;
-        uint32_t mode;
-        uint32_t size;
-        uint32_t mtime;
-    } stat_v1;
-    struct __attribute__((packed)) {
-        uint32_t id;
-        uint32_t error;
-        uint64_t dev;
-        uint64_t ino;
-        uint32_t mode;
-        uint32_t nlink;
-        uint32_t uid;
-        uint32_t gid;
-        uint64_t size;
-        int64_t atime;
-        int64_t mtime;
-        int64_t ctime;
-    } stat_v2;
-    struct __attribute__((packed)) {
-        uint32_t id;
-        uint32_t mode;
-        uint32_t size;
-        uint32_t mtime;
-        uint32_t namelen;
-    } dent_v1; // followed by `namelen` bytes of the name.
-    struct __attribute__((packed)) {
-        uint32_t id;
-        uint32_t error;
-        uint64_t dev;
-        uint64_t ino;
-        uint32_t mode;
-        uint32_t nlink;
-        uint32_t uid;
-        uint32_t gid;
-        uint64_t size;
-        int64_t atime;
-        int64_t mtime;
-        int64_t ctime;
-        uint32_t namelen;
-    } dent_v2; // followed by `namelen` bytes of the name.
-    struct __attribute__((packed)) {
-        uint32_t id;
-        uint32_t size;
-    } data; // followed by `size` bytes of data.
-    struct __attribute__((packed)) {
-        uint32_t id;
-        uint32_t msglen;
-    } status; // followed by `msglen` bytes of error message, if id == ID_FAIL.
+    sync_stat_v1 stat_v1;
+    sync_stat_v2 stat_v2;
+    sync_dent_v1 dent_v1;
+    sync_dent_v2 dent_v2;
+    sync_data data;
+    sync_status status;
 };
 
 #define SYNC_DATA_MAX (64 * 1024)
diff --git a/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h b/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h
index 957c26c..5c276b4 100644
--- a/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h
+++ b/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h
@@ -230,8 +230,14 @@
     // devices;
     // - CreateResult::ERROR if a fatal error occurred, mounting /system should
     // be aborted.
+    // This function mounts /metadata when called, and unmounts /metadata upon
+    // return.
     CreateResult RecoveryCreateSnapshotDevices();
 
+    // Same as RecoveryCreateSnapshotDevices(), but does not auto mount/umount
+    // /metadata.
+    CreateResult RecoveryCreateSnapshotDevices(const std::unique_ptr<AutoDevice>& metadata_device);
+
     // Dump debug information.
     bool Dump(std::ostream& os);
 
diff --git a/fs_mgr/libsnapshot/snapshot.cpp b/fs_mgr/libsnapshot/snapshot.cpp
index 7e84c48..2d59f0c 100644
--- a/fs_mgr/libsnapshot/snapshot.cpp
+++ b/fs_mgr/libsnapshot/snapshot.cpp
@@ -2584,6 +2584,20 @@
         LOG(ERROR) << "Couldn't mount Metadata.";
         return CreateResult::NOT_CREATED;
     }
+    return RecoveryCreateSnapshotDevices(mount);
+}
+
+CreateResult SnapshotManager::RecoveryCreateSnapshotDevices(
+        const std::unique_ptr<AutoDevice>& metadata_device) {
+    if (!device_->IsRecovery()) {
+        LOG(ERROR) << __func__ << " is only allowed in recovery.";
+        return CreateResult::NOT_CREATED;
+    }
+
+    if (metadata_device == nullptr || !metadata_device->HasDevice()) {
+        LOG(ERROR) << "Metadata not mounted.";
+        return CreateResult::NOT_CREATED;
+    }
 
     auto state_file = GetStateFilePath();
     if (access(state_file.c_str(), F_OK) != 0 && errno == ENOENT) {
diff --git a/init/Android.bp b/init/Android.bp
index 72a7bfe..d2bdf98 100644
--- a/init/Android.bp
+++ b/init/Android.bp
@@ -240,6 +240,7 @@
         "firmware_handler_test.cpp",
         "init_test.cpp",
         "keychords_test.cpp",
+        "oneshot_on_test.cpp",
         "persistent_properties_test.cpp",
         "property_service_test.cpp",
         "property_type_test.cpp",
diff --git a/init/README.md b/init/README.md
index 4f0a7ec..13f1bac 100644
--- a/init/README.md
+++ b/init/README.md
@@ -720,23 +720,35 @@
   characteristics in a device agnostic manner.
 
 Init responds to properties that begin with `ctl.`.  These properties take the format of
-`ctl.<command>` and the _value_ of the system property is used as a parameter, for example:
-`SetProperty("ctl.start", "logd")` will run the `start` command on `logd`.  Note that these
+`ctl.[<target>_]<command>` and the _value_ of the system property is used as a parameter.  The
+_target_ is optional and specifies the service option that _value_ is meant to match with.  There is
+only one option for _target_, `interface` which indicates that _value_ will refer to an interface
+that a service provides and not the service name itself.
+
+For example:
+
+`SetProperty("ctl.start", "logd")` will run the `start` command on `logd`.
+
+`SetProperty("ctl.interface_start", "aidl/aidl_lazy_test_1")` will run the `start` command on the
+service that exposes the `aidl aidl_lazy_test_1` interface.
+
+Note that these
 properties are only settable; they will have no value when read.
 
-`ctl.start` \
-`ctl.restart` \
-`ctl.stop`
-> These are equivalent to using the `start`, `restart`, and `stop` commands on the service specified
+The _commands_ are listed below.
+
+`start` \
+`restart` \
+`stop` \
+These are equivalent to using the `start`, `restart`, and `stop` commands on the service specified
 by the _value_ of the property.
 
-`ctl.interface_start` \
-`ctl.interface_restart` \
-`ctl.interface_stop`
-> These are equivalent to using the `interface_start`, `interface_restart`, and `interface_stop`
-commands on the interface specified by the _value_ of the property.
+`oneshot_one` and `oneshot_off` will turn on or off the _oneshot_
+flag for the service specified by the _value_ of the property.  This is
+particularly intended for services that are conditionally lazy HALs.  When
+they are lazy HALs, oneshot must be on, otherwise oneshot should be off.
 
-`ctl.sigstop_on` and `ctl.sigstop_off` will turn on or off the _sigstop_ feature for the service
+`sigstop_on` and `sigstop_off` will turn on or off the _sigstop_ feature for the service
 specified by the _value_ of the property.  See the _Debugging init_ section below for more details
 about this feature.
 
diff --git a/init/init.cpp b/init/init.cpp
index 4289dcf..5444a32 100644
--- a/init/init.cpp
+++ b/init/init.cpp
@@ -81,6 +81,7 @@
 using namespace std::string_literals;
 
 using android::base::boot_clock;
+using android::base::ConsumePrefix;
 using android::base::GetProperty;
 using android::base::ReadFileToString;
 using android::base::SetProperty;
@@ -367,40 +368,27 @@
     INTERFACE,  // action gets called for every service that holds this interface
 };
 
-struct ControlMessageFunction {
-    ControlTarget target;
-    std::function<Result<void>(Service*)> action;
-};
+using ControlMessageFunction = std::function<Result<void>(Service*)>;
 
-static const std::map<std::string, ControlMessageFunction>& get_control_message_map() {
+static const std::map<std::string, ControlMessageFunction, std::less<>>& GetControlMessageMap() {
     // clang-format off
-    static const std::map<std::string, ControlMessageFunction> control_message_functions = {
-        {"sigstop_on",        {ControlTarget::SERVICE,
-                               [](auto* service) { service->set_sigstop(true); return Result<void>{}; }}},
-        {"sigstop_off",       {ControlTarget::SERVICE,
-                               [](auto* service) { service->set_sigstop(false); return Result<void>{}; }}},
-        {"start",             {ControlTarget::SERVICE,   DoControlStart}},
-        {"stop",              {ControlTarget::SERVICE,   DoControlStop}},
-        {"restart",           {ControlTarget::SERVICE,   DoControlRestart}},
-        {"interface_start",   {ControlTarget::INTERFACE, DoControlStart}},
-        {"interface_stop",    {ControlTarget::INTERFACE, DoControlStop}},
-        {"interface_restart", {ControlTarget::INTERFACE, DoControlRestart}},
+    static const std::map<std::string, ControlMessageFunction, std::less<>> control_message_functions = {
+        {"sigstop_on",        [](auto* service) { service->set_sigstop(true); return Result<void>{}; }},
+        {"sigstop_off",       [](auto* service) { service->set_sigstop(false); return Result<void>{}; }},
+        {"oneshot_on",        [](auto* service) { service->set_oneshot(true); return Result<void>{}; }},
+        {"oneshot_off",       [](auto* service) { service->set_oneshot(false); return Result<void>{}; }},
+        {"start",             DoControlStart},
+        {"stop",              DoControlStop},
+        {"restart",           DoControlRestart},
     };
     // clang-format on
 
     return control_message_functions;
 }
 
-bool HandleControlMessage(const std::string& msg, const std::string& name, pid_t pid) {
-    const auto& map = get_control_message_map();
-    const auto it = map.find(msg);
-
-    if (it == map.end()) {
-        LOG(ERROR) << "Unknown control msg '" << msg << "'";
-        return false;
-    }
-
-    std::string cmdline_path = StringPrintf("proc/%d/cmdline", pid);
+static bool HandleControlMessage(std::string_view message, const std::string& name,
+                                 pid_t from_pid) {
+    std::string cmdline_path = StringPrintf("proc/%d/cmdline", from_pid);
     std::string process_cmdline;
     if (ReadFileToString(cmdline_path, &process_cmdline)) {
         std::replace(process_cmdline.begin(), process_cmdline.end(), '\0', ' ');
@@ -409,37 +397,37 @@
         process_cmdline = "unknown process";
     }
 
-    const ControlMessageFunction& function = it->second;
-
-    Service* svc = nullptr;
-
-    switch (function.target) {
-        case ControlTarget::SERVICE:
-            svc = ServiceList::GetInstance().FindService(name);
-            break;
-        case ControlTarget::INTERFACE:
-            svc = ServiceList::GetInstance().FindInterface(name);
-            break;
-        default:
-            LOG(ERROR) << "Invalid function target from static map key ctl." << msg << ": "
-                       << static_cast<std::underlying_type<ControlTarget>::type>(function.target);
-            return false;
+    Service* service = nullptr;
+    auto action = message;
+    if (ConsumePrefix(&action, "interface_")) {
+        service = ServiceList::GetInstance().FindInterface(name);
+    } else {
+        service = ServiceList::GetInstance().FindService(name);
     }
 
-    if (svc == nullptr) {
-        LOG(ERROR) << "Control message: Could not find '" << name << "' for ctl." << msg
-                   << " from pid: " << pid << " (" << process_cmdline << ")";
+    if (service == nullptr) {
+        LOG(ERROR) << "Control message: Could not find '" << name << "' for ctl." << message
+                   << " from pid: " << from_pid << " (" << process_cmdline << ")";
         return false;
     }
 
-    if (auto result = function.action(svc); !result.ok()) {
-        LOG(ERROR) << "Control message: Could not ctl." << msg << " for '" << name
-                   << "' from pid: " << pid << " (" << process_cmdline << "): " << result.error();
+    const auto& map = GetControlMessageMap();
+    const auto it = map.find(action);
+    if (it == map.end()) {
+        LOG(ERROR) << "Unknown control msg '" << message << "'";
+        return false;
+    }
+    const auto& function = it->second;
+
+    if (auto result = function(service); !result.ok()) {
+        LOG(ERROR) << "Control message: Could not ctl." << message << " for '" << name
+                   << "' from pid: " << from_pid << " (" << process_cmdline
+                   << "): " << result.error();
         return false;
     }
 
-    LOG(INFO) << "Control message: Processed ctl." << msg << " for '" << name
-              << "' from pid: " << pid << " (" << process_cmdline << ")";
+    LOG(INFO) << "Control message: Processed ctl." << message << " for '" << name
+              << "' from pid: " << from_pid << " (" << process_cmdline << ")";
     return true;
 }
 
diff --git a/init/oneshot_on_test.cpp b/init/oneshot_on_test.cpp
new file mode 100644
index 0000000..7e7cc36
--- /dev/null
+++ b/init/oneshot_on_test.cpp
@@ -0,0 +1,44 @@
+/*
+ * Copyright (C) 2020 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 <gtest/gtest.h>
+
+#include <chrono>
+
+#include <android-base/properties.h>
+
+using android::base::GetProperty;
+using android::base::SetProperty;
+using android::base::WaitForProperty;
+using namespace std::literals;
+
+TEST(init, oneshot_on) {
+    // Bootanim shouldn't be running once the device has booted.
+    ASSERT_EQ("stopped", GetProperty("init.svc.bootanim", ""));
+
+    SetProperty("ctl.oneshot_off", "bootanim");
+    SetProperty("ctl.start", "bootanim");
+
+    // Bootanim exits quickly when the device is fully booted, so check that it goes back to the
+    // 'restarting' state that non-oneshot services enter once they've restarted.
+    EXPECT_TRUE(WaitForProperty("init.svc.bootanim", "restarting", 10s));
+
+    SetProperty("ctl.oneshot_on", "bootanim");
+    SetProperty("ctl.start", "bootanim");
+
+    // Now that oneshot is enabled again, bootanim should transition into the 'stopped' state.
+    EXPECT_TRUE(WaitForProperty("init.svc.bootanim", "stopped", 10s));
+}
diff --git a/init/service.h b/init/service.h
index cf3f0c2..9f1d697 100644
--- a/init/service.h
+++ b/init/service.h
@@ -130,6 +130,13 @@
     bool is_updatable() const { return updatable_; }
     bool is_post_data() const { return post_data_; }
     bool is_from_apex() const { return from_apex_; }
+    void set_oneshot(bool value) {
+        if (value) {
+            flags_ |= SVC_ONESHOT;
+        } else {
+            flags_ &= ~SVC_ONESHOT;
+        }
+    }
 
   private:
     void NotifyStateChange(const std::string& new_state) const;
diff --git a/libstats/pull/Android.bp b/libstats/pull/Android.bp
index 1a9cb92..a7b5d91 100644
--- a/libstats/pull/Android.bp
+++ b/libstats/pull/Android.bp
@@ -53,6 +53,10 @@
         "com.android.os.statsd",
         "test_com.android.os.statsd",
     ],
+    // TODO(b/151102177): Enable it when the build error is fixed.
+    header_abi_checker: {
+        enabled: false,
+    },
 }
 
 // ONLY USE IN TESTS.
@@ -65,3 +69,25 @@
         "//frameworks/base/apex/statsd/tests/libstatspull",
     ],
 }
+
+// Note: These unit tests only test PullAtomMetadata.
+// For full E2E tests of libstatspull, use LibStatsPullTests
+cc_test {
+    name: "libstatspull_test",
+    srcs: [
+        "tests/pull_atom_metadata_test.cpp",
+    ],
+    shared_libs: [
+        "libstatspull",
+        "libstatssocket",
+    ],
+    test_suites: ["general-tests"],
+    cflags: [
+        "-Wall",
+        "-Werror",
+        "-Wno-missing-field-initializers",
+        "-Wno-unused-variable",
+        "-Wno-unused-function",
+        "-Wno-unused-parameter",
+    ],
+}
diff --git a/libstats/pull/TEST_MAPPING b/libstats/pull/TEST_MAPPING
new file mode 100644
index 0000000..76f4f02
--- /dev/null
+++ b/libstats/pull/TEST_MAPPING
@@ -0,0 +1,7 @@
+{
+  "presubmit" : [
+    {
+      "name" : "libstatspull_test"
+    }
+  ]
+}
\ No newline at end of file
diff --git a/libstats/pull/include/stats_pull_atom_callback.h b/libstats/pull/include/stats_pull_atom_callback.h
index 0b0df2b..c976c68 100644
--- a/libstats/pull/include/stats_pull_atom_callback.h
+++ b/libstats/pull/include/stats_pull_atom_callback.h
@@ -45,17 +45,27 @@
 void AStatsManager_PullAtomMetadata_release(AStatsManager_PullAtomMetadata* metadata);
 
 /**
- * Set the cool down time of the pull in nanoseconds. If two successive pulls are issued
+ * Set the cool down time of the pull in milliseconds. If two successive pulls are issued
  * within the cool down, a cached version of the first will be used for the second.
  */
-void AStatsManager_PullAtomMetadata_setCoolDownNs(AStatsManager_PullAtomMetadata* metadata,
-                                                  int64_t cool_down_ns);
+void AStatsManager_PullAtomMetadata_setCoolDownMillis(AStatsManager_PullAtomMetadata* metadata,
+                                                      int64_t cool_down_millis);
 
 /**
- * Set the maximum time the pull can take in nanoseconds.
+ * Get the cool down time of the pull in milliseconds.
  */
-void AStatsManager_PullAtomMetadata_setTimeoutNs(AStatsManager_PullAtomMetadata* metadata,
-                                                 int64_t timeout_ns);
+int64_t AStatsManager_PullAtomMetadata_getCoolDownMillis(AStatsManager_PullAtomMetadata* metadata);
+
+/**
+ * Set the maximum time the pull can take in milliseconds.
+ */
+void AStatsManager_PullAtomMetadata_setTimeoutMillis(AStatsManager_PullAtomMetadata* metadata,
+                                                     int64_t timeout_millis);
+
+/**
+ * Get the maximum time the pull can take in milliseconds.
+ */
+int64_t AStatsManager_PullAtomMetadata_getTimeoutMillis(AStatsManager_PullAtomMetadata* metadata);
 
 /**
  * Set the additive fields of this pulled atom.
@@ -65,7 +75,25 @@
  * will be combined when the non-additive fields are the same.
  */
 void AStatsManager_PullAtomMetadata_setAdditiveFields(AStatsManager_PullAtomMetadata* metadata,
-                                                      int* additive_fields, int num_fields);
+                                                      int32_t* additive_fields, int32_t num_fields);
+
+/**
+ * Get the number of additive fields for this pulled atom. This is intended to be called before
+ * AStatsManager_PullAtomMetadata_getAdditiveFields to determine the size of the array.
+ */
+int32_t AStatsManager_PullAtomMetadata_getNumAdditiveFields(
+        AStatsManager_PullAtomMetadata* metadata);
+
+/**
+ * Get the additive fields of this pulled atom.
+ *
+ * \param fields an output parameter containing the additive fields for this PullAtomMetadata.
+ *               Fields is an array and it is assumed that it is at least as large as the number of
+ *               additive fields, which can be obtained by calling
+ *               AStatsManager_PullAtomMetadata_getNumAdditiveFields.
+ */
+void AStatsManager_PullAtomMetadata_getAdditiveFields(AStatsManager_PullAtomMetadata* metadata,
+                                                      int32_t* fields);
 
 /**
  * Return codes for the result of a pull.
@@ -108,7 +136,7 @@
 typedef AStatsManager_PullAtomCallbackReturn (*AStatsManager_PullAtomCallback)(
         int32_t atom_tag, AStatsEventList* data, void* cookie);
 /**
- * Registers a callback for an atom when that atom is to be pulled. The stats service will
+ * Sets a callback for an atom when that atom is to be pulled. The stats service will
  * invoke the callback when the stats service determines that this atom needs to be
  * pulled.
  *
@@ -122,19 +150,18 @@
  * \param cookie            A pointer that will be passed back to the callback.
  *                          It has no meaning to statsd.
  */
-void AStatsManager_registerPullAtomCallback(int32_t atom_tag,
-                                            AStatsManager_PullAtomCallback callback,
-                                            AStatsManager_PullAtomMetadata* metadata, void* cookie);
+void AStatsManager_setPullAtomCallback(int32_t atom_tag, AStatsManager_PullAtomMetadata* metadata,
+                                       AStatsManager_PullAtomCallback callback, void* cookie);
 
 /**
- * Unregisters a callback for an atom when that atom is to be pulled. Note that any ongoing
+ * Clears a callback for an atom when that atom is to be pulled. Note that any ongoing
  * pulls will still occur.
  *
  * Requires the REGISTER_STATS_PULL_ATOM permission.
  *
  * \param atomTag           The tag of the atom of which to unregister
  */
-void AStatsManager_unregisterPullAtomCallback(int32_t atom_tag);
+void AStatsManager_clearPullAtomCallback(int32_t atom_tag);
 
 #ifdef __cplusplus
 }
diff --git a/libstats/pull/libstatspull.map.txt b/libstats/pull/libstatspull.map.txt
index dc3fd8b..e0e851a 100644
--- a/libstats/pull/libstatspull.map.txt
+++ b/libstats/pull/libstatspull.map.txt
@@ -2,12 +2,16 @@
     global:
         AStatsManager_PullAtomMetadata_obtain; # apex # introduced=30
         AStatsManager_PullAtomMetadata_release; # apex # introduced=30
-        AStatsManager_PullAtomMetadata_setCoolDownNs; # apex # introduced=30
-        AStatsManager_PullAtomMetadata_setTimeoutNs; # apex # introduced=30
+        AStatsManager_PullAtomMetadata_setCoolDownMillis; # apex # introduced=30
+        AStatsManager_PullAtomMetadata_getCoolDownMillis; # apex # introduced=30
+        AStatsManager_PullAtomMetadata_setTimeoutMillis; # apex # introduced=30
+        AStatsManager_PullAtomMetadata_getTimeoutMillis; # apex # introduced=30
         AStatsManager_PullAtomMetadata_setAdditiveFields; # apex # introduced=30
+        AStatsManager_PullAtomMetadata_getNumAdditiveFields; # apex # introduced=30
+        AStatsManager_PullAtomMetadata_getAdditiveFields; # apex # introduced=30
         AStatsEventList_addStatsEvent; # apex # introduced=30
-        AStatsManager_registerPullAtomCallback; # apex # introduced=30
-        AStatsManager_unregisterPullAtomCallback; # apex # introduced=30
+        AStatsManager_setPullAtomCallback; # apex # introduced=30
+        AStatsManager_clearPullAtomCallback; # apex # introduced=30
     local:
         *;
 };
diff --git a/libstats/pull/stats_pull_atom_callback.cpp b/libstats/pull/stats_pull_atom_callback.cpp
index 27e9d29..2d184bd 100644
--- a/libstats/pull/stats_pull_atom_callback.cpp
+++ b/libstats/pull/stats_pull_atom_callback.cpp
@@ -46,19 +46,19 @@
     return event;
 }
 
-static const int64_t DEFAULT_COOL_DOWN_NS = 1000000000LL;  // 1 second.
-static const int64_t DEFAULT_TIMEOUT_NS = 10000000000LL;   // 10 seconds.
+static const int64_t DEFAULT_COOL_DOWN_MILLIS = 1000LL;  // 1 second.
+static const int64_t DEFAULT_TIMEOUT_MILLIS = 10000LL;   // 10 seconds.
 
 struct AStatsManager_PullAtomMetadata {
-    int64_t cool_down_ns;
-    int64_t timeout_ns;
+    int64_t cool_down_millis;
+    int64_t timeout_millis;
     std::vector<int32_t> additive_fields;
 };
 
 AStatsManager_PullAtomMetadata* AStatsManager_PullAtomMetadata_obtain() {
     AStatsManager_PullAtomMetadata* metadata = new AStatsManager_PullAtomMetadata();
-    metadata->cool_down_ns = DEFAULT_COOL_DOWN_NS;
-    metadata->timeout_ns = DEFAULT_TIMEOUT_NS;
+    metadata->cool_down_millis = DEFAULT_COOL_DOWN_MILLIS;
+    metadata->timeout_millis = DEFAULT_TIMEOUT_MILLIS;
     metadata->additive_fields = std::vector<int32_t>();
     return metadata;
 }
@@ -67,30 +67,49 @@
     delete metadata;
 }
 
-void AStatsManager_PullAtomMetadata_setCoolDownNs(AStatsManager_PullAtomMetadata* metadata,
-                                                  int64_t cool_down_ns) {
-    metadata->cool_down_ns = cool_down_ns;
+void AStatsManager_PullAtomMetadata_setCoolDownMillis(AStatsManager_PullAtomMetadata* metadata,
+                                                      int64_t cool_down_millis) {
+    metadata->cool_down_millis = cool_down_millis;
 }
 
-void AStatsManager_PullAtomMetadata_setTimeoutNs(AStatsManager_PullAtomMetadata* metadata,
-                                                 int64_t timeout_ns) {
-    metadata->timeout_ns = timeout_ns;
+int64_t AStatsManager_PullAtomMetadata_getCoolDownMillis(AStatsManager_PullAtomMetadata* metadata) {
+    return metadata->cool_down_millis;
+}
+
+void AStatsManager_PullAtomMetadata_setTimeoutMillis(AStatsManager_PullAtomMetadata* metadata,
+                                                     int64_t timeout_millis) {
+    metadata->timeout_millis = timeout_millis;
+}
+
+int64_t AStatsManager_PullAtomMetadata_getTimeoutMillis(AStatsManager_PullAtomMetadata* metadata) {
+    return metadata->timeout_millis;
 }
 
 void AStatsManager_PullAtomMetadata_setAdditiveFields(AStatsManager_PullAtomMetadata* metadata,
-                                                      int* additive_fields, int num_fields) {
+                                                      int32_t* additive_fields,
+                                                      int32_t num_fields) {
     metadata->additive_fields.assign(additive_fields, additive_fields + num_fields);
 }
 
+int32_t AStatsManager_PullAtomMetadata_getNumAdditiveFields(
+        AStatsManager_PullAtomMetadata* metadata) {
+    return metadata->additive_fields.size();
+}
+
+void AStatsManager_PullAtomMetadata_getAdditiveFields(AStatsManager_PullAtomMetadata* metadata,
+                                                      int32_t* fields) {
+    std::copy(metadata->additive_fields.begin(), metadata->additive_fields.end(), fields);
+}
+
 class StatsPullAtomCallbackInternal : public BnPullAtomCallback {
   public:
     StatsPullAtomCallbackInternal(const AStatsManager_PullAtomCallback callback, void* cookie,
-                                  const int64_t coolDownNs, const int64_t timeoutNs,
+                                  const int64_t coolDownMillis, const int64_t timeoutMillis,
                                   const std::vector<int32_t> additiveFields)
         : mCallback(callback),
           mCookie(cookie),
-          mCoolDownNs(coolDownNs),
-          mTimeoutNs(timeoutNs),
+          mCoolDownMillis(coolDownMillis),
+          mTimeoutMillis(timeoutMillis),
           mAdditiveFields(additiveFields) {}
 
     Status onPullAtom(int32_t atomTag,
@@ -119,15 +138,15 @@
         return Status::ok();
     }
 
-    const int64_t& getCoolDownNs() const { return mCoolDownNs; }
-    const int64_t& getTimeoutNs() const { return mTimeoutNs; }
+    int64_t getCoolDownMillis() const { return mCoolDownMillis; }
+    int64_t getTimeoutMillis() const { return mTimeoutMillis; }
     const std::vector<int32_t>& getAdditiveFields() const { return mAdditiveFields; }
 
   private:
     const AStatsManager_PullAtomCallback mCallback;
     void* mCookie;
-    const int64_t mCoolDownNs;
-    const int64_t mTimeoutNs;
+    const int64_t mCoolDownMillis;
+    const int64_t mTimeoutMillis;
     const std::vector<int32_t> mAdditiveFields;
 };
 
@@ -156,8 +175,8 @@
         pullersCopy = mPullers;
     }
     for (const auto& it : pullersCopy) {
-        statsService->registerNativePullAtomCallback(it.first, it.second->getCoolDownNs(),
-                                                     it.second->getTimeoutNs(),
+        statsService->registerNativePullAtomCallback(it.first, it.second->getCoolDownMillis(),
+                                                     it.second->getTimeoutMillis(),
                                                      it.second->getAdditiveFields(), it.second);
     }
 }
@@ -186,8 +205,8 @@
         return;
     }
 
-    statsService->registerNativePullAtomCallback(atomTag, cb->getCoolDownNs(), cb->getTimeoutNs(),
-                                                 cb->getAdditiveFields(), cb);
+    statsService->registerNativePullAtomCallback(
+            atomTag, cb->getCoolDownMillis(), cb->getTimeoutMillis(), cb->getAdditiveFields(), cb);
 }
 
 void unregisterStatsPullAtomCallbackBlocking(int32_t atomTag) {
@@ -200,12 +219,11 @@
     statsService->unregisterNativePullAtomCallback(atomTag);
 }
 
-void AStatsManager_registerPullAtomCallback(int32_t atom_tag,
-                                            AStatsManager_PullAtomCallback callback,
-                                            AStatsManager_PullAtomMetadata* metadata,
-                                            void* cookie) {
-    int64_t coolDownNs = metadata == nullptr ? DEFAULT_COOL_DOWN_NS : metadata->cool_down_ns;
-    int64_t timeoutNs = metadata == nullptr ? DEFAULT_TIMEOUT_NS : metadata->timeout_ns;
+void AStatsManager_setPullAtomCallback(int32_t atom_tag, AStatsManager_PullAtomMetadata* metadata,
+                                       AStatsManager_PullAtomCallback callback, void* cookie) {
+    int64_t coolDownMillis =
+            metadata == nullptr ? DEFAULT_COOL_DOWN_MILLIS : metadata->cool_down_millis;
+    int64_t timeoutMillis = metadata == nullptr ? DEFAULT_TIMEOUT_MILLIS : metadata->timeout_millis;
 
     std::vector<int32_t> additiveFields;
     if (metadata != nullptr) {
@@ -213,8 +231,8 @@
     }
 
     std::shared_ptr<StatsPullAtomCallbackInternal> callbackBinder =
-            SharedRefBase::make<StatsPullAtomCallbackInternal>(callback, cookie, coolDownNs,
-                                                               timeoutNs, additiveFields);
+            SharedRefBase::make<StatsPullAtomCallbackInternal>(callback, cookie, coolDownMillis,
+                                                               timeoutMillis, additiveFields);
 
     {
         std::lock_guard<std::mutex> lg(pullAtomMutex);
@@ -226,7 +244,7 @@
     registerThread.detach();
 }
 
-void AStatsManager_unregisterPullAtomCallback(int32_t atom_tag) {
+void AStatsManager_clearPullAtomCallback(int32_t atom_tag) {
     {
         std::lock_guard<std::mutex> lg(pullAtomMutex);
         // Always remove the puller from our map.
diff --git a/libstats/pull/tests/pull_atom_metadata_test.cpp b/libstats/pull/tests/pull_atom_metadata_test.cpp
new file mode 100644
index 0000000..cf19303
--- /dev/null
+++ b/libstats/pull/tests/pull_atom_metadata_test.cpp
@@ -0,0 +1,92 @@
+/*
+ * Copyright (C) 2020, 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 <gtest/gtest.h>
+
+#include <stats_pull_atom_callback.h>
+
+namespace {
+
+static const int64_t DEFAULT_COOL_DOWN_MILLIS = 1000LL;  // 1 second.
+static const int64_t DEFAULT_TIMEOUT_MILLIS = 10000LL;   // 10 seconds.
+
+}  // anonymous namespace
+
+TEST(AStatsManager_PullAtomMetadataTest, TestEmpty) {
+    AStatsManager_PullAtomMetadata* metadata = AStatsManager_PullAtomMetadata_obtain();
+    EXPECT_EQ(AStatsManager_PullAtomMetadata_getCoolDownMillis(metadata), DEFAULT_COOL_DOWN_MILLIS);
+    EXPECT_EQ(AStatsManager_PullAtomMetadata_getTimeoutMillis(metadata), DEFAULT_TIMEOUT_MILLIS);
+    EXPECT_EQ(AStatsManager_PullAtomMetadata_getNumAdditiveFields(metadata), 0);
+    AStatsManager_PullAtomMetadata_release(metadata);
+}
+
+TEST(AStatsManager_PullAtomMetadataTest, TestSetTimeoutMillis) {
+    int64_t timeoutMillis = 500;
+    AStatsManager_PullAtomMetadata* metadata = AStatsManager_PullAtomMetadata_obtain();
+    AStatsManager_PullAtomMetadata_setTimeoutMillis(metadata, timeoutMillis);
+    EXPECT_EQ(AStatsManager_PullAtomMetadata_getCoolDownMillis(metadata), DEFAULT_COOL_DOWN_MILLIS);
+    EXPECT_EQ(AStatsManager_PullAtomMetadata_getTimeoutMillis(metadata), timeoutMillis);
+    EXPECT_EQ(AStatsManager_PullAtomMetadata_getNumAdditiveFields(metadata), 0);
+    AStatsManager_PullAtomMetadata_release(metadata);
+}
+
+TEST(AStatsManager_PullAtomMetadataTest, TestSetCoolDownMillis) {
+    int64_t coolDownMillis = 10000;
+    AStatsManager_PullAtomMetadata* metadata = AStatsManager_PullAtomMetadata_obtain();
+    AStatsManager_PullAtomMetadata_setCoolDownMillis(metadata, coolDownMillis);
+    EXPECT_EQ(AStatsManager_PullAtomMetadata_getCoolDownMillis(metadata), coolDownMillis);
+    EXPECT_EQ(AStatsManager_PullAtomMetadata_getTimeoutMillis(metadata), DEFAULT_TIMEOUT_MILLIS);
+    EXPECT_EQ(AStatsManager_PullAtomMetadata_getNumAdditiveFields(metadata), 0);
+    AStatsManager_PullAtomMetadata_release(metadata);
+}
+
+TEST(AStatsManager_PullAtomMetadataTest, TestSetAdditiveFields) {
+    const int numFields = 3;
+    int inputFields[numFields] = {2, 4, 6};
+    AStatsManager_PullAtomMetadata* metadata = AStatsManager_PullAtomMetadata_obtain();
+    AStatsManager_PullAtomMetadata_setAdditiveFields(metadata, inputFields, numFields);
+    EXPECT_EQ(AStatsManager_PullAtomMetadata_getCoolDownMillis(metadata), DEFAULT_COOL_DOWN_MILLIS);
+    EXPECT_EQ(AStatsManager_PullAtomMetadata_getTimeoutMillis(metadata), DEFAULT_TIMEOUT_MILLIS);
+    EXPECT_EQ(AStatsManager_PullAtomMetadata_getNumAdditiveFields(metadata), numFields);
+    int outputFields[numFields];
+    AStatsManager_PullAtomMetadata_getAdditiveFields(metadata, outputFields);
+    for (int i = 0; i < numFields; i++) {
+        EXPECT_EQ(inputFields[i], outputFields[i]);
+    }
+    AStatsManager_PullAtomMetadata_release(metadata);
+}
+
+TEST(AStatsManager_PullAtomMetadataTest, TestSetAllElements) {
+    int64_t timeoutMillis = 500;
+    int64_t coolDownMillis = 10000;
+    const int numFields = 3;
+    int inputFields[numFields] = {2, 4, 6};
+
+    AStatsManager_PullAtomMetadata* metadata = AStatsManager_PullAtomMetadata_obtain();
+    AStatsManager_PullAtomMetadata_setTimeoutMillis(metadata, timeoutMillis);
+    AStatsManager_PullAtomMetadata_setCoolDownMillis(metadata, coolDownMillis);
+    AStatsManager_PullAtomMetadata_setAdditiveFields(metadata, inputFields, numFields);
+
+    EXPECT_EQ(AStatsManager_PullAtomMetadata_getCoolDownMillis(metadata), coolDownMillis);
+    EXPECT_EQ(AStatsManager_PullAtomMetadata_getTimeoutMillis(metadata), timeoutMillis);
+    EXPECT_EQ(AStatsManager_PullAtomMetadata_getNumAdditiveFields(metadata), numFields);
+    int outputFields[numFields];
+    AStatsManager_PullAtomMetadata_getAdditiveFields(metadata, outputFields);
+    for (int i = 0; i < numFields; i++) {
+        EXPECT_EQ(inputFields[i], outputFields[i]);
+    }
+    AStatsManager_PullAtomMetadata_release(metadata);
+}
diff --git a/libunwindstack/DexFile.cpp b/libunwindstack/DexFile.cpp
index dff7a8b..bf63abf 100644
--- a/libunwindstack/DexFile.cpp
+++ b/libunwindstack/DexFile.cpp
@@ -89,7 +89,7 @@
     return nullptr;
   }
 
-  return std::unique_ptr<DexFileFromFile>(new DexFileFromFile(std::move(*art_dex_file.release())));
+  return std::unique_ptr<DexFileFromFile>(new DexFileFromFile(art_dex_file));
 }
 
 std::unique_ptr<DexFileFromMemory> DexFileFromMemory::Create(uint64_t dex_file_offset_in_memory,
@@ -108,7 +108,7 @@
 
     if (art_dex_file != nullptr) {
       return std::unique_ptr<DexFileFromMemory>(
-          new DexFileFromMemory(std::move(*art_dex_file.release()), std::move(backing_memory)));
+          new DexFileFromMemory(art_dex_file, std::move(backing_memory)));
     }
 
     if (!error_msg.empty()) {
diff --git a/libunwindstack/DexFile.h b/libunwindstack/DexFile.h
index ca658e6..4e8369f 100644
--- a/libunwindstack/DexFile.h
+++ b/libunwindstack/DexFile.h
@@ -39,7 +39,8 @@
                                          MapInfo* info);
 
  protected:
-  DexFile(art_api::dex::DexFile&& art_dex_file) : art_api::dex::DexFile(std::move(art_dex_file)) {}
+  DexFile(std::unique_ptr<art_api::dex::DexFile>& art_dex_file)
+      : art_api::dex::DexFile(art_dex_file) {}
 };
 
 class DexFileFromFile : public DexFile {
@@ -48,7 +49,7 @@
                                                  const std::string& file);
 
  private:
-  DexFileFromFile(art_api::dex::DexFile&& art_dex_file) : DexFile(std::move(art_dex_file)) {}
+  DexFileFromFile(std::unique_ptr<art_api::dex::DexFile>& art_dex_file) : DexFile(art_dex_file) {}
 };
 
 class DexFileFromMemory : public DexFile {
@@ -57,8 +58,9 @@
                                                    Memory* memory, const std::string& name);
 
  private:
-  DexFileFromMemory(art_api::dex::DexFile&& art_dex_file, std::vector<uint8_t>&& memory)
-      : DexFile(std::move(art_dex_file)), memory_(std::move(memory)) {}
+  DexFileFromMemory(std::unique_ptr<art_api::dex::DexFile>& art_dex_file,
+                    std::vector<uint8_t>&& memory)
+      : DexFile(art_dex_file), memory_(std::move(memory)) {}
 
   std::vector<uint8_t> memory_;
 };
diff --git a/libunwindstack/tests/DexFileTest.cpp b/libunwindstack/tests/DexFileTest.cpp
index 1b54da6..dc935a3 100644
--- a/libunwindstack/tests/DexFileTest.cpp
+++ b/libunwindstack/tests/DexFileTest.cpp
@@ -14,6 +14,7 @@
  * limitations under the License.
  */
 
+#include <malloc.h>
 #include <stdint.h>
 #include <sys/types.h>
 #include <unistd.h>
@@ -72,6 +73,37 @@
   EXPECT_TRUE(DexFileFromFile::Create(0x100, tf.path) != nullptr);
 }
 
+static constexpr size_t kNumLeakLoops = 5000;
+static constexpr size_t kMaxAllowedLeakBytes = 1024;
+
+static void CheckForLeak(size_t loop, size_t* first_allocated_bytes, size_t* last_allocated_bytes) {
+  size_t allocated_bytes = mallinfo().uordblks;
+  if (*first_allocated_bytes == 0) {
+    *first_allocated_bytes = allocated_bytes;
+  } else if (*last_allocated_bytes > *first_allocated_bytes) {
+    // Check that the total memory did not increase too much over the first loop.
+    ASSERT_LE(*last_allocated_bytes - *first_allocated_bytes, kMaxAllowedLeakBytes)
+        << "Failed in loop " << loop << " first_allocated_bytes " << *first_allocated_bytes
+        << " last_allocated_bytes " << *last_allocated_bytes;
+  }
+  *last_allocated_bytes = allocated_bytes;
+}
+
+TEST(DexFileTest, from_file_no_leak) {
+  TemporaryFile tf;
+  ASSERT_TRUE(tf.fd != -1);
+
+  ASSERT_EQ(sizeof(kDexData),
+            static_cast<size_t>(TEMP_FAILURE_RETRY(write(tf.fd, kDexData, sizeof(kDexData)))));
+
+  size_t first_allocated_bytes = 0;
+  size_t last_allocated_bytes = 0;
+  for (size_t i = 0; i < kNumLeakLoops; i++) {
+    EXPECT_TRUE(DexFileFromFile::Create(0, tf.path) != nullptr);
+    ASSERT_NO_FATAL_FAILURE(CheckForLeak(i, &first_allocated_bytes, &last_allocated_bytes));
+  }
+}
+
 TEST(DexFileTest, from_memory_fail_too_small_for_header) {
   MemoryFake memory;
 
@@ -96,6 +128,19 @@
   EXPECT_TRUE(DexFileFromMemory::Create(0x1000, &memory, "") != nullptr);
 }
 
+TEST(DexFileTest, from_memory_no_leak) {
+  MemoryFake memory;
+
+  memory.SetMemory(0x1000, kDexData, sizeof(kDexData));
+
+  size_t first_allocated_bytes = 0;
+  size_t last_allocated_bytes = 0;
+  for (size_t i = 0; i < kNumLeakLoops; i++) {
+    EXPECT_TRUE(DexFileFromMemory::Create(0x1000, &memory, "") != nullptr);
+    ASSERT_NO_FATAL_FAILURE(CheckForLeak(i, &first_allocated_bytes, &last_allocated_bytes));
+  }
+}
+
 TEST(DexFileTest, create_using_file) {
   TemporaryFile tf;
   ASSERT_TRUE(tf.fd != -1);