am fa76ffcc: Merge "adb: win32: fix logging to adb.log"

* commit 'fa76ffccaa723538ad09255353538911ae390322':
  adb: win32: fix logging to adb.log

adb/adb.cpp

@@ -552,6 +552,7 @@ int launch_server(int server_port)
     /* 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;
@@ -564,10 +565,40 @@ int launch_server(int server_port)
     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;
     }
 
@@ -595,9 +626,9 @@ int launch_server(int server_port)
 
     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) );
@@ -620,6 +651,8 @@ int launch_server(int server_port)
             &startup,                 /* startup info, i.e. std handles */
             &pinfo );
 
+    CloseHandle( nul_read );
+    CloseHandle( nul_write );
     CloseHandle( pipe_write );
 
     if (!ret) {

adb/client/main.cpp

@@ -110,7 +110,7 @@ static void close_stdin() {
     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 @@ static void setup_daemon_logging(void) {
     }
     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 @@ int adb_main(int is_daemon, int server_port) {
         // 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