Windows, JNI: move around sources

Move the Java JNI sources to a separate package:
c.g.devtools.build.lib.windows.jni and
c.g.devtools.build.lib.windows.runfiles.
Make the native method declarations private,
create public wrapper methods for them that ensure
that the JNI library is loaded.

Split the C++ JNI source processes.cc into two
parts (processes-jni.cc and file-jni.cc), extract
common functionality to jni-util.{h,cc}.

This change preparse the code for Android rule
support on Windows, specifically it lets the
Android BusyBox use the file JNI library so it can
create junctions on Windows to work around long
path issues when calling external tools.

See https://github.com/bazelbuild/bazel/issues/3264

Change-Id: I7f1a746d73f822ae419d11b893a91f4eb45d64da
PiperOrigin-RevId: 160643355

9 files changed, 183 insertions, 81 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/windows/WindowsFileSystem.java b/src/main/java/com/google/devtools/build/lib/windows/WindowsFileSystem.java
index e1eb08fc3b..83707d24d4 100644
--- a/src/main/java/com/google/devtools/build/lib/windows/WindowsFileSystem.java
+++ b/src/main/java/com/google/devtools/build/lib/windows/WindowsFileSystem.java
@@ -24,6 +24,7 @@ import com.google.devtools.build.lib.vfs.JavaIoFileSystem;
 import com.google.devtools.build.lib.vfs.Path;
 import com.google.devtools.build.lib.vfs.Path.PathFactory;
 import com.google.devtools.build.lib.vfs.PathFragment;
+import com.google.devtools.build.lib.windows.jni.WindowsFileOperations;
 import java.io.File;
 import java.io.FileNotFoundException;
 import java.io.IOException;
diff --git a/src/main/java/com/google/devtools/build/lib/windows/WindowsSubprocess.java b/src/main/java/com/google/devtools/build/lib/windows/WindowsSubprocess.java
index 0aea491bd3..59d9792696 100644
--- a/src/main/java/com/google/devtools/build/lib/windows/WindowsSubprocess.java
+++ b/src/main/java/com/google/devtools/build/lib/windows/WindowsSubprocess.java
@@ -15,6 +15,7 @@
 package com.google.devtools.build.lib.windows;
 
 import com.google.devtools.build.lib.shell.Subprocess;
+import com.google.devtools.build.lib.windows.jni.WindowsProcesses;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
@@ -57,7 +58,7 @@ public class WindowsSubprocess implements Subprocess {
    *
    * <p>This class is non-static for debugging purposes.
    */
-  private class ProcessInputStream extends InputStream {
+  private static final class ProcessInputStream extends InputStream {
     private long nativeStream;
 
     ProcessInputStream(long nativeStream) {
@@ -75,18 +76,18 @@ public class WindowsSubprocess implements Subprocess {
     }
 
     @Override
-    public synchronized int read(byte b[], int off, int len) throws IOException {
+    public synchronized int read(byte[] b, int off, int len) throws IOException {
       if (nativeStream == WindowsProcesses.INVALID) {
         throw new IllegalStateException();
       }
 
-      int result = WindowsProcesses.nativeReadStream(nativeStream, b, off, len);
+      int result = WindowsProcesses.readStream(nativeStream, b, off, len);
 
       if (result == 0) {
         return -1; // EOF
       }
       if (result == -1) {
-        throw new IOException(WindowsProcesses.nativeStreamGetLastError(nativeStream));
+        throw new IOException(WindowsProcesses.streamGetLastError(nativeStream));
       }
 
       return result;
@@ -95,7 +96,7 @@ public class WindowsSubprocess implements Subprocess {
     @Override
     public synchronized void close() {
       if (nativeStream != WindowsProcesses.INVALID) {
-        WindowsProcesses.nativeCloseStream(nativeStream);
+        WindowsProcesses.closeStream(nativeStream);
         nativeStream = WindowsProcesses.INVALID;
       }
     }
@@ -107,7 +108,7 @@ public class WindowsSubprocess implements Subprocess {
     }
   }
 
-  private static AtomicInteger THREAD_SEQUENCE_NUMBER = new AtomicInteger(1);
+  private static final AtomicInteger THREAD_SEQUENCE_NUMBER = new AtomicInteger(1);
   private static final ExecutorService WAITER_POOL = Executors.newCachedThreadPool(
       new ThreadFactory() {
         @Override
@@ -134,13 +135,9 @@ public class WindowsSubprocess implements Subprocess {
     this.nativeProcess = nativeProcess;
     this.timeoutMillis = timeoutMillis;
     stdoutStream =
-        stdoutRedirected
-            ? null
-            : new ProcessInputStream(WindowsProcesses.nativeGetStdout(nativeProcess));
+        stdoutRedirected ? null : new ProcessInputStream(WindowsProcesses.getStdout(nativeProcess));
     stderrStream =
-        stderrRedirected
-            ? null
-            : new ProcessInputStream(WindowsProcesses.nativeGetStderr(nativeProcess));
+        stderrRedirected ? null : new ProcessInputStream(WindowsProcesses.getStderr(nativeProcess));
     stdinStream = new ProcessOutputStream();
     waitLatch = new CountDownLatch(1);
     // Every Windows process we start consumes a thread here. This is suboptimal, but seems to be
@@ -150,7 +147,7 @@ public class WindowsSubprocess implements Subprocess {
   }
 
   private void waiterThreadFunc() {
-    switch (WindowsProcesses.nativeWaitFor(nativeProcess, timeoutMillis)) {
+    switch (WindowsProcesses.waitFor(nativeProcess, timeoutMillis)) {
       case 0:
         // Excellent, process finished in time.
         break;
@@ -158,15 +155,15 @@ public class WindowsSubprocess implements Subprocess {
       case 1:
         // Timeout. Terminate the process if we can.
         timedout.set(true);
-        WindowsProcesses.nativeTerminate(nativeProcess);
+        WindowsProcesses.terminate(nativeProcess);
         break;
 
       case 2:
         // Error. There isn't a lot we can do -- the process is still alive but
         // WaitForMultipleObjects() failed for some odd reason. We'll pretend it terminated and
         // log a message to jvm.out .
-        System.err.println("Waiting for process "
-            + WindowsProcesses.nativeGetProcessPid(nativeProcess) + " failed");
+        System.err.println(
+            "Waiting for process " + WindowsProcesses.getProcessPid(nativeProcess) + " failed");
         break;
     }
 
@@ -184,20 +181,15 @@ public class WindowsSubprocess implements Subprocess {
   @Override
   public synchronized boolean destroy() {
     checkLiveness();
-
-    if (!WindowsProcesses.nativeTerminate(nativeProcess)) {
-      return false;
-    }
-
-    return true;
+    return WindowsProcesses.terminate(nativeProcess);
   }
 
   @Override
   public synchronized int exitValue() {
     checkLiveness();
 
-    int result = WindowsProcesses.nativeGetExitCode(nativeProcess);
-    String error = WindowsProcesses.nativeProcessGetLastError(nativeProcess);
+    int result = WindowsProcesses.getExitCode(nativeProcess);
+    String error = WindowsProcesses.processGetLastError(nativeProcess);
     if (!error.isEmpty()) {
       throw new IllegalStateException(error);
     }
@@ -227,7 +219,7 @@ public class WindowsSubprocess implements Subprocess {
       stderrStream.close();
       long process = nativeProcess;
       nativeProcess = WindowsProcesses.INVALID;
-      WindowsProcesses.nativeDeleteProcess(process);
+      WindowsProcesses.deleteProcess(process);
     }
   }
 
@@ -252,12 +244,11 @@ public class WindowsSubprocess implements Subprocess {
     int remaining = len;
     int currentOffset = off;
     while (remaining != 0) {
-      int written = WindowsProcesses.nativeWriteStdin(
-          nativeProcess, b, currentOffset, remaining);
+      int written = WindowsProcesses.writeStdin(nativeProcess, b, currentOffset, remaining);
       // I think the Windows API never returns 0 in dwNumberOfBytesWritten
       // Verify.verify(written != 0);
       if (written == -1) {
-        throw new IOException(WindowsProcesses.nativeProcessGetLastError(nativeProcess));
+        throw new IOException(WindowsProcesses.processGetLastError(nativeProcess));
       }
 
       remaining -= written;
diff --git a/src/main/java/com/google/devtools/build/lib/windows/WindowsSubprocessFactory.java b/src/main/java/com/google/devtools/build/lib/windows/WindowsSubprocessFactory.java
index e85325dcaa..e398a3b195 100644
--- a/src/main/java/com/google/devtools/build/lib/windows/WindowsSubprocessFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/windows/WindowsSubprocessFactory.java
@@ -19,6 +19,7 @@ import com.google.devtools.build.lib.shell.Subprocess;
 import com.google.devtools.build.lib.shell.SubprocessBuilder;
 import com.google.devtools.build.lib.shell.SubprocessBuilder.StreamAction;
 import com.google.devtools.build.lib.vfs.PathFragment;
+import com.google.devtools.build.lib.windows.jni.WindowsProcesses;
 import java.io.File;
 import java.io.IOException;
 import java.util.List;
@@ -37,11 +38,9 @@ public class WindowsSubprocessFactory implements Subprocess.Factory {
 
   @Override
   public Subprocess create(SubprocessBuilder builder) throws IOException {
-    WindowsJniLoader.loadJni();
-
     List<String> argv = builder.getArgv();
 
-    // DO NOT quote argv0, nativeCreateProcess will do it for us.
+    // DO NOT quote argv0, createProcess will do it for us.
     String argv0 = processArgv0(argv.get(0));
     String argvRest =
         argv.size() > 1 ? WindowsProcesses.quoteCommandLine(argv.subList(1, argv.size())) : "";
@@ -51,11 +50,11 @@ public class WindowsSubprocessFactory implements Subprocess.Factory {
     String stderrPath = getRedirectPath(builder.getStderr(), builder.getStderrFile());
 
     long nativeProcess =
-        WindowsProcesses.nativeCreateProcess(
+        WindowsProcesses.createProcess(
             argv0, argvRest, env, builder.getWorkingDirectory().getPath(), stdoutPath, stderrPath);
-    String error = WindowsProcesses.nativeProcessGetLastError(nativeProcess);
+    String error = WindowsProcesses.processGetLastError(nativeProcess);
     if (!error.isEmpty()) {
-      WindowsProcesses.nativeDeleteProcess(nativeProcess);
+      WindowsProcesses.deleteProcess(nativeProcess);
       throw new IOException(error);
     }
 
diff --git a/src/main/java/com/google/devtools/build/lib/windows/jni/BUILD b/src/main/java/com/google/devtools/build/lib/windows/jni/BUILD
new file mode 100644
index 0000000000..ed8ca69799
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/windows/jni/BUILD
@@ -0,0 +1,37 @@
+package(
+    default_visibility = [
+        "//src/main/java/com/google/devtools/build/lib:__subpackages__",
+        "//src/test/java/com/google/devtools/build/lib:__subpackages__",
+    ],
+)
+
+filegroup(
+    name = "srcs",
+    srcs = glob(["**"]),
+)
+
+java_library(
+    name = "jni",
+    exports = [
+        ":file",
+        ":processes",
+    ],
+)
+
+java_library(
+    name = "file",
+    srcs = ["WindowsFileOperations.java"],
+    deps = [":jni-loader"],
+)
+
+java_library(
+    name = "processes",
+    srcs = ["WindowsProcesses.java"],
+    deps = [":jni-loader"],
+)
+
+java_library(
+    name = "jni-loader",
+    srcs = ["WindowsJniLoader.java"],
+    deps = ["//src/main/java/com/google/devtools/build/lib/windows/runfiles"],
+)
diff --git a/src/main/java/com/google/devtools/build/lib/windows/WindowsFileOperations.java b/src/main/java/com/google/devtools/build/lib/windows/jni/WindowsFileOperations.java
index 0d68c36db8..44b6cba976 100644
--- a/src/main/java/com/google/devtools/build/lib/windows/WindowsFileOperations.java
+++ b/src/main/java/com/google/devtools/build/lib/windows/jni/WindowsFileOperations.java
@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-package com.google.devtools.build.lib.windows;
+package com.google.devtools.build.lib.windows.jni;
 
 import java.io.IOException;
 
@@ -49,11 +49,11 @@ public class WindowsFileOperations {
   private static final int IS_JUNCTION_NO = 1;
   private static final int IS_JUNCTION_ERROR = 2;
 
-  static native int nativeIsJunction(String path, String[] error);
+  private static native int nativeIsJunction(String path, String[] error);
 
-  static native boolean nativeGetLongPath(String path, String[] result, String[] error);
+  private static native boolean nativeGetLongPath(String path, String[] result, String[] error);
 
-  static native boolean nativeCreateJunction(String name, String target, String[] error);
+  private static native boolean nativeCreateJunction(String name, String target, String[] error);
 
   /** Determines whether `path` is a junction point or directory symlink. */
   public static boolean isJunction(String path) throws IOException {
diff --git a/src/main/java/com/google/devtools/build/lib/windows/WindowsJniLoader.java b/src/main/java/com/google/devtools/build/lib/windows/jni/WindowsJniLoader.java
index bc59eacca4..8b53d0c1a8 100644
--- a/src/main/java/com/google/devtools/build/lib/windows/WindowsJniLoader.java
+++ b/src/main/java/com/google/devtools/build/lib/windows/jni/WindowsJniLoader.java
@@ -12,15 +12,15 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-package com.google.devtools.build.lib.windows;
+package com.google.devtools.build.lib.windows.jni;
 
+import com.google.devtools.build.lib.windows.runfiles.WindowsRunfiles;
 import java.io.IOException;
 
-/**
- * Loads native code under Windows.
- */
+/** Loads native code under Windows. */
 public class WindowsJniLoader {
   private static boolean jniLoaded = false;
+
   public static synchronized void loadJni() {
     if (jniLoaded) {
       return;
diff --git a/src/main/java/com/google/devtools/build/lib/windows/WindowsProcesses.java b/src/main/java/com/google/devtools/build/lib/windows/jni/WindowsProcesses.java
index 9aac2a7719..1eaab25904 100644
--- a/src/main/java/com/google/devtools/build/lib/windows/WindowsProcesses.java
+++ b/src/main/java/com/google/devtools/build/lib/windows/jni/WindowsProcesses.java
@@ -12,13 +12,11 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-package com.google.devtools.build.lib.windows;
+package com.google.devtools.build.lib.windows.jni;
 
 import java.util.List;
 
-/**
- * Process management on Windows.
- */
+/** Process management on Windows. */
 public class WindowsProcesses {
   public static final long INVALID = -1;
 
@@ -27,11 +25,6 @@ public class WindowsProcesses {
   }
 
   /**
-   * returns the PID of the current process.
-   */
-  static native int nativeGetpid();
-
-  /**
    * Creates a process with the specified Windows command line.
    *
    * <p>Appropriately quoting arguments is the responsibility of the caller.
@@ -50,7 +43,13 @@ public class WindowsProcesses {
    *     work.
    * @return the opaque identifier of the created process
    */
-  static native long nativeCreateProcess(
+  public static long createProcess(
+      String argv0, String argvRest, byte[] env, String cwd, String stdoutFile, String stderrFile) {
+    WindowsJniLoader.loadJni();
+    return nativeCreateProcess(argv0, argvRest, env, cwd, stdoutFile, stderrFile);
+  }
+
+  private static native long nativeCreateProcess(
       String argv0, String argvRest, byte[] env, String cwd, String stdoutFile, String stderrFile);
 
   /**
@@ -60,13 +59,28 @@ public class WindowsProcesses {
    *
    * @return the number of bytes written
    */
-  static native int nativeWriteStdin(long process, byte[] bytes, int offset, int length);
+  public static int writeStdin(long process, byte[] bytes, int offset, int length) {
+    WindowsJniLoader.loadJni();
+    return nativeWriteStdin(process, bytes, offset, length);
+  }
+
+  private static native int nativeWriteStdin(long process, byte[] bytes, int offset, int length);
 
   /** Returns an opaque identifier of stdout stream for the process. */
-  static native long nativeGetStdout(long process);
+  public static long getStdout(long process) {
+    WindowsJniLoader.loadJni();
+    return nativeGetStdout(process);
+  }
+
+  private static native long nativeGetStdout(long process);
 
-  /** Returns am opaque identifier of stderr stream for the process. */
-  static native long nativeGetStderr(long process);
+  /** Returns an opaque identifier of stderr stream for the process. */
+  public static long getStderr(long process) {
+    WindowsJniLoader.loadJni();
+    return nativeGetStderr(process);
+  }
+
+  private static native long nativeGetStderr(long process);
 
   /**
    * Reads data from the stream into the given array. {@code stream} should come from {@link
@@ -76,34 +90,55 @@ public class WindowsProcesses {
    *
    * @return the number of bytes read, 0 on EOF, or -1 if there was an error.
    */
-  static native int nativeReadStream(long stream, byte[] bytes, int offset, int length);
+  public static int readStream(long stream, byte[] bytes, int offset, int length) {
+    WindowsJniLoader.loadJni();
+    return nativeReadStream(stream, bytes, offset, length);
+  }
+
+  private static native int nativeReadStream(long stream, byte[] bytes, int offset, int length);
 
   /**
    * Waits until the given process terminates. If timeout is non-negative, it indicates the number
    * of milliseconds before the call times out.
    *
    * <p>Return values:
-   * <li>0: Process finished</li>
-   * <li>1: Timeout</li>
-   * <li>2: Something went wrong</li>
+   * <li>0: Process finished
+   * <li>1: Timeout
+   * <li>2: Something went wrong
    */
-  static native int nativeWaitFor(long process, long timeout);
+  public static int waitFor(long process, long timeout) {
+    WindowsJniLoader.loadJni();
+    return nativeWaitFor(process, timeout);
+  }
 
-  /**
-   * Returns the exit code of the process. Throws {@code IllegalStateException} if something
-   * goes wrong.
-   */
-  static native int nativeGetExitCode(long process);
+  private static native int nativeWaitFor(long process, long timeout);
 
   /**
-   * Returns the process ID of the given process or -1 if there was an error.
+   * Returns the exit code of the process. Throws {@code IllegalStateException} if something goes
+   * wrong.
    */
-  static native int nativeGetProcessPid(long process);
+  public static int getExitCode(long process) {
+    WindowsJniLoader.loadJni();
+    return nativeGetExitCode(process);
+  }
 
-  /**
-   * Terminates the given process. Returns true if the termination was successful.
-   */
-  static native boolean nativeTerminate(long process);
+  private static native int nativeGetExitCode(long process);
+
+  /** Returns the process ID of the given process or -1 if there was an error. */
+  public static int getProcessPid(long process) {
+    WindowsJniLoader.loadJni();
+    return nativeGetProcessPid(process);
+  }
+
+  private static native int nativeGetProcessPid(long process);
+
+  /** Terminates the given process. Returns true if the termination was successful. */
+  public static boolean terminate(long process) {
+    WindowsJniLoader.loadJni();
+    return nativeTerminate(process);
+  }
+
+  private static native boolean nativeTerminate(long process);
 
   /**
    * Releases the native data structures associated with the process.
@@ -111,7 +146,12 @@ public class WindowsProcesses {
    * <p>Calling any other method on the same process after this call will result in the JVM crashing
    * or worse.
    */
-  static native void nativeDeleteProcess(long process);
+  public static void deleteProcess(long process) {
+    WindowsJniLoader.loadJni();
+    nativeDeleteProcess(process);
+  }
+
+  private static native void nativeDeleteProcess(long process);
 
   /**
    * Closes the stream
@@ -119,7 +159,12 @@ public class WindowsProcesses {
    * @param stream should come from {@link #nativeGetStdout(long)} or {@link
    *     #nativeGetStderr(long)}.
    */
-  static native void nativeCloseStream(long stream);
+  public static void closeStream(long stream) {
+    WindowsJniLoader.loadJni();
+    nativeCloseStream(stream);
+  }
+
+  private static native void nativeCloseStream(long stream);
 
   /**
    * Returns a string representation of the last error caused by any call on the given process or
@@ -130,16 +175,29 @@ public class WindowsProcesses {
    * <p>After this call returns, subsequent calls will return the empty string if there was no
    * failed operation in between.
    */
-  static native String nativeProcessGetLastError(long process);
+  public static String processGetLastError(long process) {
+    WindowsJniLoader.loadJni();
+    return nativeProcessGetLastError(process);
+  }
 
-  static native String nativeStreamGetLastError(long process);
+  private static native String nativeProcessGetLastError(long process);
 
+  public static String streamGetLastError(long process) {
+    WindowsJniLoader.loadJni();
+    return nativeStreamGetLastError(process);
+  }
+
+  private static native String nativeStreamGetLastError(long process);
+
+  /** returns the PID of the current process. */
   public static int getpid() {
     WindowsJniLoader.loadJni();
     return nativeGetpid();
   }
 
-  static String quoteCommandLine(List<String> argv) {
+  private static native int nativeGetpid();
+
+  public static String quoteCommandLine(List<String> argv) {
     StringBuilder result = new StringBuilder();
     for (int iArg = 0; iArg < argv.size(); iArg++) {
       if (iArg != 0) {
@@ -170,8 +228,8 @@ public class WindowsProcesses {
               } else {
                 // Backslashes everywhere else are quoted if they are followed by a
                 // quote or a backslash
-                result.append(arg.charAt(iChar + 1) == '"' || arg.charAt(iChar + 1) == '\\'
-                    ? "\\\\" : "\\");
+                result.append(
+                    arg.charAt(iChar + 1) == '"' || arg.charAt(iChar + 1) == '\\' ? "\\\\" : "\\");
               }
               break;
             default:
diff --git a/src/main/java/com/google/devtools/build/lib/windows/runfiles/BUILD b/src/main/java/com/google/devtools/build/lib/windows/runfiles/BUILD
new file mode 100644
index 0000000000..1ec264d53f
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/windows/runfiles/BUILD
@@ -0,0 +1,16 @@
+package(
+    default_visibility = [
+        "//src/main/java/com/google/devtools/build/lib:__subpackages__",
+        "//src/test/java/com/google/devtools/build/lib:__subpackages__",
+    ],
+)
+
+filegroup(
+    name = "srcs",
+    srcs = glob(["**"]),
+)
+
+java_library(
+    name = "runfiles",
+    srcs = ["WindowsRunfiles.java"],
+)
diff --git a/src/main/java/com/google/devtools/build/lib/windows/WindowsRunfiles.java b/src/main/java/com/google/devtools/build/lib/windows/runfiles/WindowsRunfiles.java
index 876531073a..bf0d131259 100644
--- a/src/main/java/com/google/devtools/build/lib/windows/WindowsRunfiles.java
+++ b/src/main/java/com/google/devtools/build/lib/windows/runfiles/WindowsRunfiles.java
@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-package com.google.devtools.build.lib.windows;
+package com.google.devtools.build.lib.windows.runfiles;
 
 import java.io.BufferedReader;
 import java.io.FileInputStream;
@@ -59,7 +59,7 @@ public final class WindowsRunfiles {
           }
 
           runfiles.put(splitLine[0], splitLine[1]);
-        }   
+        }
       }
     }
   }