Merge "adb: call run_transport_disconnects() only once."
diff --git a/adb/adb.cpp b/adb/adb.cpp
index d6e6d91..49cf123 100644
--- a/adb/adb.cpp
+++ b/adb/adb.cpp
@@ -580,6 +580,105 @@
#if ADB_HOST
+#ifdef _WIN32
+
+static bool _make_handle_noninheritable(HANDLE h) {
+ if (h != INVALID_HANDLE_VALUE && h != NULL) {
+ if (!SetHandleInformation(h, HANDLE_FLAG_INHERIT, 0)) {
+ // Show the handle value to give us a clue in case we have problems
+ // with pseudo-handle values.
+ fprintf(stderr, "Cannot make handle 0x%p non-inheritable: %s\n",
+ h, SystemErrorCodeToString(GetLastError()).c_str());
+ return false;
+ }
+ }
+
+ return true;
+}
+
+// Create anonymous pipe, preventing inheritance of the read pipe and setting
+// security of the write pipe to sa.
+static bool _create_anonymous_pipe(unique_handle* pipe_read_out,
+ unique_handle* pipe_write_out,
+ SECURITY_ATTRIBUTES* sa) {
+ HANDLE pipe_read_raw = NULL;
+ HANDLE pipe_write_raw = NULL;
+ if (!CreatePipe(&pipe_read_raw, &pipe_write_raw, sa, 0)) {
+ fprintf(stderr, "Cannot create pipe: %s\n",
+ SystemErrorCodeToString(GetLastError()).c_str());
+ return false;
+ }
+
+ unique_handle pipe_read(pipe_read_raw);
+ pipe_read_raw = NULL;
+ unique_handle pipe_write(pipe_write_raw);
+ pipe_write_raw = NULL;
+
+ if (!_make_handle_noninheritable(pipe_read.get())) {
+ return false;
+ }
+
+ *pipe_read_out = std::move(pipe_read);
+ *pipe_write_out = std::move(pipe_write);
+
+ return true;
+}
+
+// Read from a pipe (that we take ownership of) and write what is returned to
+// GetStdHandle(nStdHandle). Return on error or when the pipe is closed.
+static unsigned _redirect_pipe_thread(HANDLE h, DWORD nStdHandle) {
+ // Take ownership of the HANDLE and close when we're done.
+ unique_handle read_pipe(h);
+ const HANDLE write_handle = GetStdHandle(nStdHandle);
+ const char* output_name = nStdHandle == STD_OUTPUT_HANDLE ?
+ "stdout" : "stderr";
+
+ while (true) {
+ char buf[64 * 1024];
+ DWORD bytes_read = 0;
+ if (!ReadFile(read_pipe.get(), buf, sizeof(buf), &bytes_read, NULL)) {
+ const DWORD err = GetLastError();
+ // ERROR_BROKEN_PIPE is expected when the subprocess closes
+ // the other end of the pipe.
+ if (err == ERROR_BROKEN_PIPE) {
+ return EXIT_SUCCESS;
+ } else {
+ fprintf(stderr, "Failed to read from %s: %s\n", output_name,
+ SystemErrorCodeToString(err).c_str());
+ return EXIT_FAILURE;
+ }
+ }
+
+ // Don't try to write if our stdout/stderr was not setup by the
+ // parent process.
+ if (write_handle != NULL && write_handle != INVALID_HANDLE_VALUE) {
+ DWORD bytes_written = 0;
+ if (!WriteFile(write_handle, buf, bytes_read, &bytes_written,
+ NULL)) {
+ fprintf(stderr, "Failed to write to %s: %s\n", output_name,
+ SystemErrorCodeToString(GetLastError()).c_str());
+ return EXIT_FAILURE;
+ }
+
+ if (bytes_written != bytes_read) {
+ fprintf(stderr, "Only wrote %lu of %lu bytes to %s\n",
+ bytes_written, bytes_read, output_name);
+ return EXIT_FAILURE;
+ }
+ }
+ }
+}
+
+static unsigned __stdcall _redirect_stdout_thread(HANDLE h) {
+ return _redirect_pipe_thread(h, STD_OUTPUT_HANDLE);
+}
+
+static unsigned __stdcall _redirect_stderr_thread(HANDLE h) {
+ return _redirect_pipe_thread(h, STD_ERROR_HANDLE);
+}
+
+#endif
+
int launch_server(int server_port)
{
#if defined(_WIN32)
@@ -587,58 +686,42 @@
/* we create a PIPE that will be used to wait for the server's "OK" */
/* message since the pipe handles must be inheritable, we use a */
/* security attribute */
- HANDLE nul_read, nul_write;
- HANDLE pipe_read, pipe_write;
- HANDLE stdout_handle, stderr_handle;
SECURITY_ATTRIBUTES sa;
- STARTUPINFOW startup;
- PROCESS_INFORMATION pinfo;
- WCHAR program_path[ MAX_PATH ];
- int ret;
-
sa.nLength = sizeof(sa);
sa.lpSecurityDescriptor = NULL;
sa.bInheritHandle = TRUE;
- /* Redirect stdin and stderr to Windows /dev/null. If we instead pass our
- * stdin/stderr handles and they are console handles, when the adb server
- * starts up, the C Runtime will see console handles for a process that
- * isn't connected to a console and it will configure stderr to be closed.
- * At that point, freopen() could be used to reopen stderr, but it would
- * take more massaging to fixup the file descriptor number that freopen()
- * uses. It's simplest to avoid all of this complexity by just redirecting
- * stdin/stderr to `nul' and then the C Runtime acts as expected.
- */
- nul_read = CreateFileW(L"nul", GENERIC_READ,
- FILE_SHARE_READ | FILE_SHARE_WRITE, &sa,
- OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL);
- if (nul_read == INVALID_HANDLE_VALUE) {
- fprintf(stderr, "CreateFileW(nul, GENERIC_READ) failed: %s\n",
+ // Redirect stdin to Windows /dev/null. If we instead pass an original
+ // stdin/stdout/stderr handle and it is a console handle, when the adb
+ // server starts up, the C Runtime will see a console handle for a process
+ // that isn't connected to a console and it will configure
+ // stdin/stdout/stderr to be closed. At that point, freopen() could be used
+ // to reopen stderr/out, but it would take more massaging to fixup the file
+ // descriptor number that freopen() uses. It's simplest to avoid all of this
+ // complexity by just redirecting stdin to `nul' and then the C Runtime acts
+ // as expected.
+ unique_handle nul_read(CreateFileW(L"nul", GENERIC_READ,
+ FILE_SHARE_READ | FILE_SHARE_WRITE, &sa, OPEN_EXISTING,
+ FILE_ATTRIBUTE_NORMAL, NULL));
+ if (nul_read.get() == INVALID_HANDLE_VALUE) {
+ fprintf(stderr, "Cannot open 'nul': %s\n",
SystemErrorCodeToString(GetLastError()).c_str());
return -1;
}
- nul_write = CreateFileW(L"nul", GENERIC_WRITE,
- FILE_SHARE_READ | FILE_SHARE_WRITE, &sa,
- OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL);
- if (nul_write == INVALID_HANDLE_VALUE) {
- fprintf(stderr, "CreateFileW(nul, GENERIC_WRITE) failed: %s\n",
- SystemErrorCodeToString(GetLastError()).c_str());
- CloseHandle(nul_read);
+ // create pipes with non-inheritable read handle, inheritable write handle
+ unique_handle ack_read, ack_write;
+ if (!_create_anonymous_pipe(&ack_read, &ack_write, &sa)) {
return -1;
}
-
- /* create pipe, and ensure its read handle isn't inheritable */
- ret = CreatePipe( &pipe_read, &pipe_write, &sa, 0 );
- if (!ret) {
- fprintf(stderr, "CreatePipe() failed: %s\n",
- SystemErrorCodeToString(GetLastError()).c_str());
- CloseHandle(nul_read);
- CloseHandle(nul_write);
+ unique_handle stdout_read, stdout_write;
+ if (!_create_anonymous_pipe(&stdout_read, &stdout_write, &sa)) {
return -1;
}
-
- SetHandleInformation( pipe_read, HANDLE_FLAG_INHERIT, 0 );
+ unique_handle stderr_read, stderr_write;
+ if (!_create_anonymous_pipe(&stderr_read, &stderr_write, &sa)) {
+ return -1;
+ }
/* Some programs want to launch an adb command and collect its output by
* calling CreateProcess with inheritable stdout/stderr handles, then
@@ -650,52 +733,64 @@
* the calling process is stuck while read()-ing from the stdout/stderr
* descriptors, because they're connected to corresponding handles in the
* adb server process (even if the latter never uses/writes to them).
+ * Note that even if we don't pass these handles in the STARTUPINFO struct,
+ * if they're marked inheritable, they're still inherited, requiring us to
+ * deal with this.
+ *
+ * If we're still having problems with inheriting random handles in the
+ * future, consider using PROC_THREAD_ATTRIBUTE_HANDLE_LIST to explicitly
+ * specify which handles should be inherited: http://blogs.msdn.com/b/oldnewthing/archive/2011/12/16/10248328.aspx
*/
- stdout_handle = GetStdHandle( STD_OUTPUT_HANDLE );
- stderr_handle = GetStdHandle( STD_ERROR_HANDLE );
- if (stdout_handle != INVALID_HANDLE_VALUE) {
- SetHandleInformation( stdout_handle, HANDLE_FLAG_INHERIT, 0 );
+ if (!_make_handle_noninheritable(GetStdHandle(STD_INPUT_HANDLE))) {
+ return -1;
}
- if (stderr_handle != INVALID_HANDLE_VALUE) {
- SetHandleInformation( stderr_handle, HANDLE_FLAG_INHERIT, 0 );
+ if (!_make_handle_noninheritable(GetStdHandle(STD_OUTPUT_HANDLE))) {
+ return -1;
+ }
+ if (!_make_handle_noninheritable(GetStdHandle(STD_ERROR_HANDLE))) {
+ return -1;
}
+ STARTUPINFOW startup;
ZeroMemory( &startup, sizeof(startup) );
startup.cb = sizeof(startup);
- startup.hStdInput = nul_read;
- startup.hStdOutput = nul_write;
- startup.hStdError = nul_write;
+ startup.hStdInput = nul_read.get();
+ startup.hStdOutput = stdout_write.get();
+ startup.hStdError = stderr_write.get();
startup.dwFlags = STARTF_USESTDHANDLES;
- ZeroMemory( &pinfo, sizeof(pinfo) );
+ // Verify that the pipe_write handle value can be passed on the command line
+ // as %d and that the rest of adb code can pass it around in an int.
+ const int ack_write_as_int = cast_handle_to_int(ack_write.get());
+ if (cast_int_to_handle(ack_write_as_int) != ack_write.get()) {
+ // If this fires, either handle values are larger than 32-bits or else
+ // there is a bug in our casting.
+ // https://msdn.microsoft.com/en-us/library/windows/desktop/aa384203%28v=vs.85%29.aspx
+ fprintf(stderr, "Cannot fit pipe handle value into 32-bits: 0x%p\n",
+ ack_write.get());
+ return -1;
+ }
- /* get path of current program */
- DWORD module_result = GetModuleFileNameW(NULL, program_path,
- arraysize(program_path));
- if ((module_result == arraysize(program_path)) || (module_result == 0)) {
+ // get path of current program
+ WCHAR program_path[MAX_PATH];
+ const DWORD module_result = GetModuleFileNameW(NULL, program_path,
+ arraysize(program_path));
+ if ((module_result >= arraysize(program_path)) || (module_result == 0)) {
// String truncation or some other error.
- fprintf(stderr, "GetModuleFileNameW() failed: %s\n",
+ fprintf(stderr, "Cannot get executable path: %s\n",
SystemErrorCodeToString(GetLastError()).c_str());
return -1;
}
- // Verify that the pipe_write handle value can be passed on the command line
- // as %d and that the rest of adb code can pass it around in an int.
- const int pipe_write_as_int = cast_handle_to_int(pipe_write);
- if (cast_int_to_handle(pipe_write_as_int) != pipe_write) {
- // If this fires, either handle values are larger than 32-bits or else
- // there is a bug in our casting.
- // https://msdn.microsoft.com/en-us/library/windows/desktop/aa384203%28v=vs.85%29.aspx
- fprintf(stderr, "CreatePipe handle value too large: 0x%p\n",
- pipe_write);
- return -1;
- }
-
- WCHAR args[64];
+ WCHAR args[64];
snwprintf(args, arraysize(args),
L"adb -P %d fork-server server --reply-fd %d", server_port,
- pipe_write_as_int);
- ret = CreateProcessW(
+ ack_write_as_int);
+
+ PROCESS_INFORMATION pinfo;
+ ZeroMemory(&pinfo, sizeof(pinfo));
+
+ if (!CreateProcessW(
program_path, /* program path */
args,
/* the fork-server argument will set the
@@ -707,38 +802,113 @@
NULL, /* use parent's environment block */
NULL, /* use parent's starting directory */
&startup, /* startup info, i.e. std handles */
- &pinfo );
-
- CloseHandle( nul_read );
- CloseHandle( nul_write );
- CloseHandle( pipe_write );
-
- if (!ret) {
- fprintf(stderr, "CreateProcess failed: %s\n",
+ &pinfo )) {
+ fprintf(stderr, "Cannot create process: %s\n",
SystemErrorCodeToString(GetLastError()).c_str());
- CloseHandle( pipe_read );
return -1;
}
- CloseHandle( pinfo.hProcess );
- CloseHandle( pinfo.hThread );
+ unique_handle process_handle(pinfo.hProcess);
+ pinfo.hProcess = NULL;
- /* wait for the "OK\n" message */
+ // Close handles that we no longer need to complete the rest.
+ CloseHandle(pinfo.hThread);
+ pinfo.hThread = NULL;
+
+ nul_read.reset();
+ ack_write.reset();
+ stdout_write.reset();
+ stderr_write.reset();
+
+ // Start threads to read from subprocess stdout/stderr and write to ours
+ // to make subprocess errors easier to diagnose.
+
+ // In the past, reading from a pipe before the child process's C Runtime
+ // started up and called GetFileType() caused a hang: http://blogs.msdn.com/b/oldnewthing/archive/2011/12/02/10243553.aspx#10244216
+ // This is reportedly fixed in Windows Vista: https://support.microsoft.com/en-us/kb/2009703
+ // I was unable to reproduce the problem on Windows XP. It sounds like a
+ // Windows Update may have fixed this: https://www.duckware.com/tech/peeknamedpipe.html
+ unique_handle stdout_thread(reinterpret_cast<HANDLE>(
+ _beginthreadex(NULL, 0, _redirect_stdout_thread, stdout_read.get(),
+ 0, NULL)));
+ if (stdout_thread.get() == nullptr) {
+ fprintf(stderr, "Cannot create thread: %s\n", strerror(errno));
+ return -1;
+ }
+ stdout_read.release(); // Transfer ownership to new thread
+
+ unique_handle stderr_thread(reinterpret_cast<HANDLE>(
+ _beginthreadex(NULL, 0, _redirect_stderr_thread, stderr_read.get(),
+ 0, NULL)));
+ if (stderr_thread.get() == nullptr) {
+ fprintf(stderr, "Cannot create thread: %s\n", strerror(errno));
+ return -1;
+ }
+ stderr_read.release(); // Transfer ownership to new thread
+
+ bool got_ack = false;
+
+ // Wait for the "OK\n" message, for the pipe to be closed, or other error.
{
- char temp[3];
- DWORD count;
+ char temp[3];
+ DWORD count = 0;
- ret = ReadFile( pipe_read, temp, 3, &count, NULL );
- CloseHandle( pipe_read );
- if ( !ret ) {
- fprintf(stderr, "could not read ok from ADB Server, error: %s\n",
- SystemErrorCodeToString(GetLastError()).c_str());
- return -1;
+ if (ReadFile(ack_read.get(), temp, sizeof(temp), &count, NULL)) {
+ const CHAR expected[] = "OK\n";
+ const DWORD expected_length = arraysize(expected) - 1;
+ if (count == expected_length &&
+ memcmp(temp, expected, expected_length) == 0) {
+ got_ack = true;
+ } else {
+ fprintf(stderr, "ADB server didn't ACK\n");
+ }
+ } else {
+ const DWORD err = GetLastError();
+ // If the ACK was not written and the process exited, GetLastError()
+ // is probably ERROR_BROKEN_PIPE, in which case that info is not
+ // useful to the user.
+ fprintf(stderr, "could not read ok from ADB Server%s\n",
+ err == ERROR_BROKEN_PIPE ? "" :
+ android::base::StringPrintf(": %s",
+ SystemErrorCodeToString(err).c_str()).c_str());
}
- if (count != 3 || temp[0] != 'O' || temp[1] != 'K' || temp[2] != '\n') {
- fprintf(stderr, "ADB server didn't ACK\n" );
- return -1;
+ }
+
+ // Always try to wait a bit for threads reading stdout/stderr to finish.
+ // If the process started ok, it should close the pipes causing the threads
+ // to finish. If the process had an error, it should exit, also causing
+ // the pipes to be closed. In that case we want to read all of the output
+ // and write it out so that the user can diagnose failures.
+ const DWORD thread_timeout_ms = 15 * 1000;
+ const HANDLE threads[] = { stdout_thread.get(), stderr_thread.get() };
+ const DWORD wait_result = WaitForMultipleObjects(arraysize(threads),
+ threads, TRUE, thread_timeout_ms);
+ if (wait_result == WAIT_TIMEOUT) {
+ // Threads did not finish after waiting a little while. Perhaps the
+ // server didn't close pipes, or it is hung.
+ fprintf(stderr, "Timed-out waiting for threads to finish reading from "
+ "ADB Server\n");
+ // Process handles are signaled when the process exits, so if we wait
+ // on the handle for 0 seconds and it returns 'timeout', that means that
+ // the process is still running.
+ if (WaitForSingleObject(process_handle.get(), 0) == WAIT_TIMEOUT) {
+ // We could TerminateProcess(), but that seems somewhat presumptive.
+ fprintf(stderr, "ADB Server is running: process id %lu\n",
+ pinfo.dwProcessId);
}
+ return -1;
+ }
+
+ if (wait_result != WAIT_OBJECT_0) {
+ fprintf(stderr, "Unexpected result waiting for threads: %lu: %s\n",
+ wait_result, SystemErrorCodeToString(GetLastError()).c_str());
+ return -1;
+ }
+
+ // For now ignore the thread exit codes and assume they worked properly.
+
+ if (!got_ack) {
+ return -1;
}
#else /* !defined(_WIN32) */
char path[PATH_MAX];
diff --git a/adb/client/main.cpp b/adb/client/main.cpp
index 39bb02b..f6ddeb4 100644
--- a/adb/client/main.cpp
+++ b/adb/client/main.cpp
@@ -76,6 +76,8 @@
static const char kNullFileName[] = "NUL";
static BOOL WINAPI ctrlc_handler(DWORD type) {
+ // TODO: Consider trying to kill a starting up adb server (if we're in
+ // launch_server) by calling GenerateConsoleCtrlEvent().
exit(STATUS_CONTROL_C_EXIT);
return TRUE;
}
@@ -128,17 +130,26 @@
}
unix_close(fd);
-#ifdef _WIN32
- // On Windows, stderr is buffered by default, so switch to non-buffered
- // to match Linux.
- setvbuf(stderr, NULL, _IONBF, 0);
-#endif
fprintf(stderr, "--- adb starting (pid %d) ---\n", getpid());
LOG(INFO) << adb_version();
}
int adb_main(int is_daemon, int server_port, int ack_reply_fd) {
#if defined(_WIN32)
+ // adb start-server starts us up with stdout and stderr hooked up to
+ // anonymous pipes to. When the C Runtime sees this, it makes stderr and
+ // stdout buffered, but to improve the chance that error output is seen,
+ // unbuffer stdout and stderr just like if we were run at the console.
+ // This also keeps stderr unbuffered when it is redirected to adb.log.
+ if (is_daemon) {
+ if (setvbuf(stdout, NULL, _IONBF, 0) == -1) {
+ fatal("cannot make stdout unbuffered: %s", strerror(errno));
+ }
+ if (setvbuf(stderr, NULL, _IONBF, 0) == -1) {
+ fatal("cannot make stderr unbuffered: %s", strerror(errno));
+ }
+ }
+
SetConsoleCtrlHandler(ctrlc_handler, TRUE);
#else
signal(SIGPIPE, SIG_IGN);
@@ -162,6 +173,12 @@
// Inform our parent that we are up and running.
if (is_daemon) {
+ close_stdin();
+ setup_daemon_logging();
+
+ // Any error output written to stderr now goes to adb.log. We could
+ // keep around a copy of the stderr fd and use that to write any errors
+ // encountered by the following code, but that is probably overkill.
#if defined(_WIN32)
const HANDLE ack_reply_handle = cast_int_to_handle(ack_reply_fd);
const CHAR ack[] = "OK\n";
@@ -184,8 +201,6 @@
}
unix_close(ack_reply_fd);
#endif
- close_stdin();
- setup_daemon_logging();
}
D("Event loop starting\n");
diff --git a/adb/device.py b/adb/device.py
index 5b33ff2..c5b5eea 100644
--- a/adb/device.py
+++ b/adb/device.py
@@ -17,6 +17,7 @@
import os
import re
import subprocess
+import tempfile
class FindDeviceError(RuntimeError):
@@ -99,6 +100,36 @@
return _get_unique_device(product)
+# Call this instead of subprocess.check_output() to work-around issue in Python
+# 2's subprocess class on Windows where it doesn't support Unicode. This
+# writes the command line to a UTF-8 batch file that is properly interpreted
+# by cmd.exe.
+def _subprocess_check_output(*popenargs, **kwargs):
+ # Only do this slow work-around if Unicode is in the cmd line.
+ if (os.name == 'nt' and
+ any(isinstance(arg, unicode) for arg in popenargs[0])):
+ # cmd.exe requires a suffix to know that it is running a batch file
+ tf = tempfile.NamedTemporaryFile('wb', suffix='.cmd', delete=False)
+ # @ in batch suppresses echo of the current line.
+ # Change the codepage to 65001, the UTF-8 codepage.
+ tf.write('@chcp 65001 > nul\r\n')
+ tf.write('@')
+ # Properly quote all the arguments and encode in UTF-8.
+ tf.write(subprocess.list2cmdline(popenargs[0]).encode('utf-8'))
+ tf.close()
+
+ try:
+ result = subprocess.check_output(['cmd.exe', '/c', tf.name],
+ **kwargs)
+ except subprocess.CalledProcessError as e:
+ # Show real command line instead of the cmd.exe command line.
+ raise subprocess.CalledProcessError(e.returncode, popenargs[0],
+ output=e.output)
+ finally:
+ os.remove(tf.name)
+ return result
+ else:
+ return subprocess.check_output(*popenargs, **kwargs)
class AndroidDevice(object):
# Delimiter string to indicate the start of the exit code.
@@ -166,13 +197,13 @@
def _simple_call(self, cmd):
logging.info(' '.join(self.adb_cmd + cmd))
- return subprocess.check_output(
+ return _subprocess_check_output(
self.adb_cmd + cmd, stderr=subprocess.STDOUT)
def shell(self, cmd):
logging.info(' '.join(self.adb_cmd + ['shell'] + cmd))
cmd = self._make_shell_cmd(cmd)
- out = subprocess.check_output(cmd)
+ out = _subprocess_check_output(cmd)
rc, out = self._parse_shell_output(out)
if rc != 0:
error = subprocess.CalledProcessError(rc, cmd)
diff --git a/adb/file_sync_client.cpp b/adb/file_sync_client.cpp
index e70d550..94876ee 100644
--- a/adb/file_sync_client.cpp
+++ b/adb/file_sync_client.cpp
@@ -179,39 +179,41 @@
return sync_start_stat(sc, path) && sync_finish_stat(sc, timestamp, mode, size);
}
-static int write_data_file(SyncConnection& sc, const char* path, syncsendbuf* sbuf, bool show_progress) {
- int err = 0;
+static bool write_data_file(SyncConnection& sc, const char* path, syncsendbuf* sbuf, bool show_progress) {
unsigned long long size = 0;
+ if (show_progress) {
+ // Determine local file size.
+ struct stat st;
+ if (stat(path, &st) == -1) {
+ fprintf(stderr, "cannot stat '%s': %s\n", path, strerror(errno));
+ return false;
+ }
+
+ size = st.st_size;
+ }
int lfd = adb_open(path, O_RDONLY);
if (lfd < 0) {
fprintf(stderr, "cannot open '%s': %s\n", path, strerror(errno));
- return -1;
- }
-
- if (show_progress) {
- // Determine local file size.
- struct stat st;
- if (stat(path, &st)) {
- fprintf(stderr, "cannot stat '%s': %s\n", path, strerror(errno));
- return -1;
- }
-
- size = st.st_size;
+ return false;
}
sbuf->id = ID_DATA;
while (true) {
int ret = adb_read(lfd, sbuf->data, sc.max);
if (ret <= 0) {
- if (ret < 0) fprintf(stderr, "cannot read '%s': %s\n", path, strerror(errno));
+ if (ret < 0) {
+ fprintf(stderr, "cannot read '%s': %s\n", path, strerror(errno));
+ adb_close(lfd);
+ return false;
+ }
break;
}
sbuf->size = ret;
if (!WriteFdExactly(sc.fd, sbuf, sizeof(unsigned) * 2 + ret)) {
- err = -1;
- break;
+ adb_close(lfd);
+ return false;
}
sc.total_bytes += ret;
@@ -221,17 +223,17 @@
}
adb_close(lfd);
- return err;
+ return true;
}
#if defined(_WIN32)
-extern int write_data_link(SyncConnection& sc, const char* path, syncsendbuf* sbuf) __attribute__((error("no symlinks on Windows")));
+extern bool write_data_link(SyncConnection& sc, const char* path, syncsendbuf* sbuf) __attribute__((error("no symlinks on Windows")));
#else
-static int write_data_link(SyncConnection& sc, const char* path, syncsendbuf* sbuf) {
+static bool write_data_link(SyncConnection& sc, const char* path, syncsendbuf* sbuf) {
ssize_t len = readlink(path, sbuf->data, sc.max - 1);
if (len < 0) {
fprintf(stderr, "error reading link '%s': %s\n", path, strerror(errno));
- return -1;
+ return false;
}
sbuf->data[len] = '\0';
@@ -239,12 +241,12 @@
sbuf->id = ID_DATA;
if (!WriteFdExactly(sc.fd, sbuf, sizeof(unsigned) * 2 + len + 1)) {
- return -1;
+ return false;
}
sc.total_bytes += len + 1;
- return 0;
+ return true;
}
#endif
@@ -257,9 +259,9 @@
if (!SendRequest(sc.fd, ID_SEND, path_and_mode.c_str())) goto fail;
if (S_ISREG(mode)) {
- write_data_file(sc, lpath, sbuf, show_progress);
+ if (!write_data_file(sc, lpath, sbuf, show_progress)) return false;
} else if (S_ISLNK(mode)) {
- write_data_link(sc, lpath, sbuf);
+ if (!write_data_link(sc, lpath, sbuf)) return false;
} else {
goto fail;
}
@@ -325,6 +327,7 @@
while (true) {
if(!ReadFdExactly(sc.fd, &msg.data, sizeof(msg.data))) {
+ adb_close(lfd);
return -1;
}
id = msg.data.id;
diff --git a/adb/sockets.cpp b/adb/sockets.cpp
index a85d5ad..9c13936 100644
--- a/adb/sockets.cpp
+++ b/adb/sockets.cpp
@@ -471,14 +471,6 @@
}
#endif /* ADB_HOST */
-/* a Remote socket is used to send/receive data to/from a given transport object
-** it needs to be closed when the transport is forcibly destroyed by the user
-*/
-struct aremotesocket {
- asocket socket;
- adisconnect disconnect;
-};
-
static int remote_socket_enqueue(asocket *s, apacket *p)
{
D("entered remote_socket_enqueue RS(%d) WRITE fd=%d peer.fd=%d\n",
@@ -526,33 +518,17 @@
D("entered remote_socket_close RS(%d) CLOSE fd=%d peer->fd=%d\n",
s->id, s->fd, s->peer?s->peer->fd:-1);
D("RS(%d): closed\n", s->id);
- remove_transport_disconnect( s->transport, &((aremotesocket*)s)->disconnect );
free(s);
}
-static void remote_socket_disconnect(void* _s, atransport* t)
-{
- asocket* s = reinterpret_cast<asocket*>(_s);
- asocket* peer = s->peer;
-
- D("remote_socket_disconnect RS(%d)\n", s->id);
- if (peer) {
- peer->peer = NULL;
- peer->close(peer);
- }
- remove_transport_disconnect( s->transport, &((aremotesocket*)s)->disconnect );
- free(s);
-}
-
-/* Create an asocket to exchange packets with a remote service through transport
- |t|. Where |id| is the socket id of the corresponding service on the other
- side of the transport (it is allocated by the remote side and _cannot_ be 0).
- Returns a new non-NULL asocket handle. */
+// Create a remote socket to exchange packets with a remote service through transport
+// |t|. Where |id| is the socket id of the corresponding service on the other
+// side of the transport (it is allocated by the remote side and _cannot_ be 0).
+// Returns a new non-NULL asocket handle.
asocket *create_remote_socket(unsigned id, atransport *t)
{
if (id == 0) fatal("invalid remote socket id (0)");
- asocket* s = reinterpret_cast<asocket*>(calloc(1, sizeof(aremotesocket)));
- adisconnect* dis = &reinterpret_cast<aremotesocket*>(s)->disconnect;
+ asocket* s = reinterpret_cast<asocket*>(calloc(1, sizeof(asocket)));
if (s == NULL) fatal("cannot allocate socket");
s->id = id;
@@ -562,9 +538,6 @@
s->close = remote_socket_close;
s->transport = t;
- dis->func = remote_socket_disconnect;
- dis->opaque = s;
- add_transport_disconnect( t, dis );
D("RS(%d): created\n", s->id);
return s;
}
diff --git a/adb/sysdeps.h b/adb/sysdeps.h
index 6f3c443..a58a762 100644
--- a/adb/sysdeps.h
+++ b/adb/sysdeps.h
@@ -71,6 +71,7 @@
#include <windows.h>
#include <ws2tcpip.h>
+#include <memory> // unique_ptr
#include <string> // Prototypes for narrow() and widen() use std::(w)string.
#include "fdevent.h"
@@ -355,6 +356,21 @@
return reinterpret_cast<HANDLE>(static_cast<INT_PTR>(fd));
}
+// Deleter for unique_handle. Adapted from many sources, including:
+// http://stackoverflow.com/questions/14841396/stdunique-ptr-deleters-and-the-win32-api
+// https://visualstudiomagazine.com/articles/2013/09/01/get-a-handle-on-the-windows-api.aspx
+class handle_deleter {
+public:
+ typedef HANDLE pointer;
+
+ void operator()(HANDLE h);
+};
+
+// Like std::unique_ptr, but for Windows HANDLE objects that should be
+// CloseHandle()'d. Operator bool() only checks if the handle != nullptr,
+// but does not check if the handle != INVALID_HANDLE_VALUE.
+typedef std::unique_ptr<HANDLE, handle_deleter> unique_handle;
+
#else /* !_WIN32 a.k.a. Unix */
#include "fdevent.h"
diff --git a/adb/sysdeps_win32.cpp b/adb/sysdeps_win32.cpp
index f534d61..015f89e 100644
--- a/adb/sysdeps_win32.cpp
+++ b/adb/sysdeps_win32.cpp
@@ -112,6 +112,22 @@
return msg;
}
+void handle_deleter::operator()(HANDLE h) {
+ // CreateFile() is documented to return INVALID_HANDLE_FILE on error,
+ // implying that NULL is a valid handle, but this is probably impossible.
+ // Other APIs like CreateEvent() are documented to return NULL on error,
+ // implying that INVALID_HANDLE_VALUE is a valid handle, but this is also
+ // probably impossible. Thus, consider both NULL and INVALID_HANDLE_VALUE
+ // as invalid handles. std::unique_ptr won't call a deleter with NULL, so we
+ // only need to check for INVALID_HANDLE_VALUE.
+ if (h != INVALID_HANDLE_VALUE) {
+ if (!CloseHandle(h)) {
+ D("CloseHandle(%p) failed: %s\n", h,
+ SystemErrorCodeToString(GetLastError()).c_str());
+ }
+ }
+}
+
/**************************************************************************/
/**************************************************************************/
/***** *****/
diff --git a/adb/test_device.py b/adb/test_device.py
index c893ad4..2006937 100644
--- a/adb/test_device.py
+++ b/adb/test_device.py
@@ -215,12 +215,18 @@
def test_install_argument_escaping(self):
"""Make sure that install argument escaping works."""
# http://b/20323053
- tf = tempfile.NamedTemporaryFile('wb', suffix='-text;ls;1.apk')
+ tf = tempfile.NamedTemporaryFile('wb', suffix='-text;ls;1.apk',
+ delete=False)
+ tf.close()
self.assertIn("-text;ls;1.apk", self.device.install(tf.name))
+ os.remove(tf.name)
# http://b/3090932
- tf = tempfile.NamedTemporaryFile('wb', suffix="-Live Hold'em.apk")
+ tf = tempfile.NamedTemporaryFile('wb', suffix="-Live Hold'em.apk",
+ delete=False)
+ tf.close()
self.assertIn("-Live Hold'em.apk", self.device.install(tf.name))
+ os.remove(tf.name)
class RootUnrootTest(DeviceTest):
@@ -451,19 +457,24 @@
def test_unicode_paths(self):
"""Ensure that we can support non-ASCII paths, even on Windows."""
- name = u'로보카 폴리'.encode('utf-8')
+ name = u'로보카 폴리'
## push.
- tf = tempfile.NamedTemporaryFile('wb', suffix=name)
- self.device.push(tf.name, '/data/local/tmp/adb-test-{}'.format(name))
+ tf = tempfile.NamedTemporaryFile('wb', suffix=name, delete=False)
+ tf.close()
+ self.device.push(tf.name, u'/data/local/tmp/adb-test-{}'.format(name))
+ os.remove(tf.name)
self.device.shell(['rm', '-f', '/data/local/tmp/adb-test-*'])
# pull.
- cmd = ['touch', '"/data/local/tmp/adb-test-{}"'.format(name)]
+ cmd = ['touch', u'"/data/local/tmp/adb-test-{}"'.format(name)]
self.device.shell(cmd)
- tf = tempfile.NamedTemporaryFile('wb', suffix=name)
- self.device.pull('/data/local/tmp/adb-test-{}'.format(name), tf.name)
+ tf = tempfile.NamedTemporaryFile('wb', suffix=name, delete=False)
+ tf.close()
+ self.device.pull(u'/data/local/tmp/adb-test-{}'.format(name), tf.name)
+ os.remove(tf.name)
+ self.device.shell(['rm', '-f', '/data/local/tmp/adb-test-*'])
def main():