[adb] Fix incremental installation on Windows
Use only the syscalls that work with the wrapped ADB fds, or
extract the native handles for the case when need to call one
not wrapped.
Bug: 151239696
Test: adb install --incremental <apk> on Windows
Change-Id: Ia6de620171ab696b8136dcb60a2b63af6f86419f
diff --git a/adb/client/commandline.cpp b/adb/client/commandline.cpp
index 14ccfbe..2199fd3 100644
--- a/adb/client/commandline.cpp
+++ b/adb/client/commandline.cpp
@@ -1423,13 +1423,13 @@
#endif
}
-static bool _is_valid_fd(int fd) {
+static bool _is_valid_os_fd(int fd) {
// Disallow invalid FDs and stdin/out/err as well.
if (fd < 3) {
return false;
}
#ifdef _WIN32
- HANDLE handle = adb_get_os_handle(fd);
+ auto handle = (HANDLE)fd;
DWORD info = 0;
if (GetHandleInformation(handle, &info) == 0) {
return false;
@@ -2005,7 +2005,7 @@
#endif
}
int connection_fd = atoi(argv[1]);
- if (!_is_valid_fd(connection_fd)) {
+ if (!_is_valid_os_fd(connection_fd)) {
error_exit("Invalid connection_fd number given: %d", connection_fd);
}
@@ -2013,7 +2013,7 @@
close_on_exec(connection_fd);
int output_fd = atoi(argv[2]);
- if (!_is_valid_fd(output_fd)) {
+ if (!_is_valid_os_fd(output_fd)) {
error_exit("Invalid output_fd number given: %d", output_fd);
}
output_fd = adb_register_socket(output_fd);
diff --git a/adb/client/incremental.cpp b/adb/client/incremental.cpp
index fd608cc..a9e65dc 100644
--- a/adb/client/incremental.cpp
+++ b/adb/client/incremental.cpp
@@ -41,8 +41,7 @@
static inline int32_t read_int32(borrowed_fd fd) {
int32_t result;
- ReadFully(fd, &result, sizeof(result));
- return result;
+ return ReadFdExactly(fd, &result, sizeof(result)) ? result : -1;
}
static inline void append_int(borrowed_fd fd, std::vector<char>* bytes) {
@@ -54,11 +53,14 @@
static inline void append_bytes_with_size(borrowed_fd fd, std::vector<char>* bytes) {
int32_t le_size = read_int32(fd);
+ if (le_size < 0) {
+ return;
+ }
int32_t size = int32_t(le32toh(le_size));
auto old_size = bytes->size();
bytes->resize(old_size + sizeof(le_size) + size);
memcpy(bytes->data() + old_size, &le_size, sizeof(le_size));
- ReadFully(fd, bytes->data() + old_size + sizeof(le_size), size);
+ ReadFdExactly(fd, bytes->data() + old_size + sizeof(le_size), size);
}
static inline std::pair<std::vector<char>, int32_t> read_id_sig_headers(borrowed_fd fd) {
@@ -200,7 +202,7 @@
return {};
}
auto [pipe_read_fd, pipe_write_fd] = print_fds;
- auto pipe_write_fd_param = std::to_string(pipe_write_fd);
+ auto pipe_write_fd_param = std::to_string(intptr_t(adb_get_os_handle(pipe_write_fd)));
close_on_exec(pipe_read_fd);
std::vector<std::string> args(std::move(files));
diff --git a/adb/client/incremental_utils.cpp b/adb/client/incremental_utils.cpp
index 316480c..caadb26 100644
--- a/adb/client/incremental_utils.cpp
+++ b/adb/client/incremental_utils.cpp
@@ -18,6 +18,7 @@
#include "incremental_utils.h"
+#include <android-base/mapped_file.h>
#include <android-base/strings.h>
#include <ziparchive/zip_archive.h>
#include <ziparchive/zip_writer.h>
@@ -26,6 +27,7 @@
#include <numeric>
#include <unordered_set>
+#include "adb_trace.h"
#include "sysdeps.h"
static constexpr int kBlockSize = 4096;
@@ -155,18 +157,81 @@
return zipPriorityBlocks;
}
-// TODO(b/151676293): avoid using OpenArchiveFd that reads local file headers
+[[maybe_unused]] static ZipArchiveHandle openZipArchiveFd(int fd) {
+ bool transferFdOwnership = false;
+#ifdef _WIN32
+ //
+ // Need to create a special CRT FD here as the current one is not compatible with
+ // normal read()/write() calls that libziparchive uses.
+ // To make this work we have to create a copy of the file handle, as CRT doesn't care
+ // and closes it together with the new descriptor.
+ //
+ // Note: don't move this into a helper function, it's better to be hard to reuse because
+ // the code is ugly and won't work unless it's a last resort.
+ //
+ auto handle = adb_get_os_handle(fd);
+ HANDLE dupedHandle;
+ if (!::DuplicateHandle(::GetCurrentProcess(), handle, ::GetCurrentProcess(), &dupedHandle, 0,
+ false, DUPLICATE_SAME_ACCESS)) {
+ D("%s failed at DuplicateHandle: %d", __func__, (int)::GetLastError());
+ return {};
+ }
+ fd = _open_osfhandle((intptr_t)dupedHandle, _O_RDONLY | _O_BINARY);
+ if (fd < 0) {
+ D("%s failed at _open_osfhandle: %d", __func__, errno);
+ ::CloseHandle(handle);
+ return {};
+ }
+ transferFdOwnership = true;
+#endif
+ ZipArchiveHandle zip;
+ if (OpenArchiveFd(fd, "apk_fd", &zip, transferFdOwnership) != 0) {
+ D("%s failed at OpenArchiveFd: %d", __func__, errno);
+#ifdef _WIN32
+ // "_close()" is a secret WinCRT name for the regular close() function.
+ _close(fd);
+#endif
+ return {};
+ }
+ return zip;
+}
+
+static std::pair<ZipArchiveHandle, std::unique_ptr<android::base::MappedFile>> openZipArchive(
+ int fd, int64_t fileSize) {
+#ifndef __LP64__
+ if (fileSize >= INT_MAX) {
+ return {openZipArchiveFd(fd), nullptr};
+ }
+#endif
+ auto mapping =
+ android::base::MappedFile::FromOsHandle(adb_get_os_handle(fd), 0, fileSize, PROT_READ);
+ if (!mapping) {
+ D("%s failed at FromOsHandle: %d", __func__, errno);
+ return {};
+ }
+ ZipArchiveHandle zip;
+ if (OpenArchiveFromMemory(mapping->data(), mapping->size(), "apk_mapping", &zip) != 0) {
+ D("%s failed at OpenArchiveFromMemory: %d", __func__, errno);
+ return {};
+ }
+ return {zip, std::move(mapping)};
+}
+
+// TODO(b/151676293): avoid using libziparchive as it reads local file headers
// which causes additional performance cost. Instead, only read from central directory.
static std::vector<int32_t> InstallationPriorityBlocks(int fd, int64_t fileSize) {
- std::vector<int32_t> installationPriorityBlocks;
- ZipArchiveHandle zip;
- if (OpenArchiveFd(fd, "", &zip, false) != 0) {
+ auto [zip, _] = openZipArchive(fd, fileSize);
+ if (!zip) {
return {};
}
+
void* cookie = nullptr;
if (StartIteration(zip, &cookie) != 0) {
+ D("%s failed at StartIteration: %d", __func__, errno);
return {};
}
+
+ std::vector<int32_t> installationPriorityBlocks;
ZipEntry entry;
std::string_view entryName;
while (Next(cookie, &entry, &entryName) == 0) {
@@ -183,6 +248,7 @@
int32_t endBlockIndex = offsetToBlockIndex(entryEndOffset);
int32_t numNewBlocks = endBlockIndex - startBlockIndex + 1;
appendBlocks(startBlockIndex, numNewBlocks, &installationPriorityBlocks);
+ D("\tadding to priority blocks: '%.*s'", (int)entryName.size(), entryName.data());
} else if (entryName == "classes.dex") {
// Only the head is needed for installation
int32_t startBlockIndex = offsetToBlockIndex(entry.offset);
@@ -213,4 +279,4 @@
unduplicate(priorityBlocks);
return priorityBlocks;
}
-} // namespace incremental
\ No newline at end of file
+} // namespace incremental
diff --git a/adb/sysdeps.h b/adb/sysdeps.h
index 4efbc02..3e781b8 100644
--- a/adb/sysdeps.h
+++ b/adb/sysdeps.h
@@ -276,6 +276,7 @@
class Process {
public:
constexpr explicit Process(HANDLE h = nullptr) : h_(h) {}
+ constexpr Process(Process&& other) : h_(std::exchange(other.h_, nullptr)) {}
~Process() { close(); }
constexpr explicit operator bool() const { return h_ != nullptr; }
@@ -292,6 +293,8 @@
}
private:
+ DISALLOW_COPY_AND_ASSIGN(Process);
+
void close() {
if (*this) {
::CloseHandle(h_);
@@ -666,6 +669,8 @@
class Process {
public:
constexpr explicit Process(pid_t pid) : pid_(pid) {}
+ constexpr Process(Process&& other) : pid_(std::exchange(other.pid_, -1)) {}
+
constexpr explicit operator bool() const { return pid_ >= 0; }
void wait() {
@@ -682,6 +687,8 @@
}
private:
+ DISALLOW_COPY_AND_ASSIGN(Process);
+
pid_t pid_;
};