adb: win32: fix logging to adb.log
In the adb client, redirect stdin and stderr of the adb server to `nul',
so that when the adb server starts up, it avoids issues in the C Runtime
where it closes stderr, making it hard to properly reopen. There are
probably other ways to avoid this issue, but I think this is the
cleanest that will keep working over the years and will exercise the
most commonly used code-paths in the C Runtime.
Fix some adb_close() calls to be unix_close() (only really matters on
Windows).
Make stderr non-buffered on Windows, to match the (sensible) Linux
behavior.
Change-Id: I1b15c64240e50dbeb56788b0d0d901f4536ad788
Signed-off-by: Spencer Low <CompareAndSwap@gmail.com>
diff --git a/adb/adb.cpp b/adb/adb.cpp
index 9c1ead5..fee5361 100644
--- a/adb/adb.cpp
+++ b/adb/adb.cpp
@@ -545,6 +545,7 @@
/* 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;
@@ -557,10 +558,40 @@
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) failure, error %ld\n",
+ GetLastError());
+ 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) failure, error %ld\n",
+ GetLastError());
+ CloseHandle(nul_read);
+ 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() failure, error %ld\n", GetLastError() );
+ CloseHandle(nul_read);
+ CloseHandle(nul_write);
return -1;
}
@@ -588,9 +619,9 @@
ZeroMemory( &startup, sizeof(startup) );
startup.cb = sizeof(startup);
- startup.hStdInput = GetStdHandle( STD_INPUT_HANDLE );
+ startup.hStdInput = nul_read;
startup.hStdOutput = pipe_write;
- startup.hStdError = GetStdHandle( STD_ERROR_HANDLE );
+ startup.hStdError = nul_write;
startup.dwFlags = STARTF_USESTDHANDLES;
ZeroMemory( &pinfo, sizeof(pinfo) );
@@ -613,6 +644,8 @@
&startup, /* startup info, i.e. std handles */
&pinfo );
+ CloseHandle( nul_read );
+ CloseHandle( nul_write );
CloseHandle( pipe_write );
if (!ret) {
diff --git a/adb/client/main.cpp b/adb/client/main.cpp
index 0cd6670..c018b8a 100644
--- a/adb/client/main.cpp
+++ b/adb/client/main.cpp
@@ -110,7 +110,7 @@
int fd = unix_open(kNullFileName, O_RDONLY);
CHECK_NE(fd, -1);
dup2(fd, STDIN_FILENO);
- adb_close(fd);
+ unix_close(fd);
}
static void setup_daemon_logging(void) {
@@ -121,7 +121,13 @@
}
dup2(fd, STDOUT_FILENO);
dup2(fd, STDERR_FILENO);
- adb_close(fd);
+ 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());
}
@@ -153,7 +159,15 @@
// Inform our parent that we are up and running.
// TODO(danalbert): Can't use SendOkay because we're sending "OK\n", not
// "OKAY".
- // TODO(danalbert): Why do we use stdout for Windows?
+ // TODO(danalbert): Why do we use stdout for Windows? There is a
+ // comment in launch_server() that suggests that non-Windows uses
+ // stderr because it is non-buffered. So perhaps the history is that
+ // stdout was preferred for all platforms, but it was discovered that
+ // non-Windows needed a non-buffered fd, so stderr was used there.
+ // Note that using stderr on unix means that if you do
+ // `ADB_TRACE=all adb start-server`, it will say "ADB server didn't ACK"
+ // and "* failed to start daemon *" because the adb server will write
+ // logging to stderr, obscuring the OK\n output that is sent to stderr.
#if defined(_WIN32)
int reply_fd = STDOUT_FILENO;
// Change stdout mode to binary so \n => \r\n translation does not
