Issue jenkinsci#29 - Do not retrieve Environment variables just to ge…

…t the command line

It does not resolve the issue, but it reduces its scope to API users, which actually require Environment Variables.
Unfortunately JENKINS-30782 is of such kind, because Jenkins process termination logic for builds relies on the environment variables.

native/envvar-cmdline.cpp

@@ -75,8 +75,20 @@ static inline LPVOID Ptr32ToPtr64(PTR32(LPVOID) v) {
 }
 #endif
 
+jstring getCmdLineAndEnvVars(JNIEnv* pEnv, jclass clazz, jint pid, jint retrieveEnvVars);
+
+JNIEXPORT jstring JNICALL Java_org_jvnet_winp_Native_getCmdLine(
+	JNIEnv* pEnv, jclass clazz, jint pid) {
+	return getCmdLineAndEnvVars(pEnv, clazz, pid, 0);
+}
+
 JNIEXPORT jstring JNICALL Java_org_jvnet_winp_Native_getCmdLineAndEnvVars(
 	JNIEnv* pEnv, jclass clazz, jint pid) {
+	return getCmdLineAndEnvVars(pEnv, clazz, pid, 1);
+}
+
+jstring getCmdLineAndEnvVars(
+	JNIEnv* pEnv, jclass clazz, jint pid, jint retrieveEnvVars){
 
 	// see http://msdn2.microsoft.com/en-us/library/ms674678%28VS.85%29.aspx
 	// for kernel string functions
@@ -176,14 +188,20 @@ JNIEXPORT jstring JNICALL Java_org_jvnet_winp_Native_getCmdLineAndEnvVars(
 	}	// end of !wow64 code
 #endif
 
+	int cmdLineLen = lstrlen(pszCmdLine);
+	if (retrieveEnvVars == 0) {
+		// No need to retrieve Environment Variables
+		jstring packedStr = pEnv->NewString((jchar*)(LPWSTR)pszCmdLine, cmdLineLen);
+		return packedStr;
+	}
+
 	// figure out the size of the env var block
 	MEMORY_BASIC_INFORMATION info;
 	if(VirtualQueryEx(hProcess, pEnvStr, &info, sizeof(info))==0) {
 		reportError(pEnv, "VirtualQueryEx failed");
 		return NULL;
 	}
-
-	int cmdLineLen = lstrlen(pszCmdLine);
+
 	size_t envSize = info.RegionSize - ((char*)pEnvStr - (char*)info.BaseAddress);
 
 	auto_localmem<LPWSTR> buf((cmdLineLen + 1/*for \0*/) * 2 + envSize);

src/main/java/org/jvnet/winp/Native.java

@@ -29,12 +29,21 @@ class Native {
     /**
      * Gets the command line and environment variables of the process
      * identified by the process ID.
+     * If the environment variables are not required, consider using {@link #getCmdLine(int)}.
      *
      * <p>
      * To simplify the JNI side, the resulting string is structured to
      * "cmdlineargs\0env1=val1\0env2=val2\0..."
      */
     native static String getCmdLineAndEnvVars(int pid);
+
+    /**
+     * Gets the command line of the process identified by the process ID.
+     * @param pid Process ID
+     * @return Command line or {@code null} if it cannot be retrieved
+     * @throws WinpException Operation failure
+     */
+    native static String getCmdLine(int pid) throws WinpException;
 
     /**
      * Enumerate all processes.

src/main/java/org/jvnet/winp/WinProcess.java

@@ -103,8 +103,9 @@ public void setPriority(int priority) {
      *      maybe we didn't have enough security privileges.
      */
     public synchronized String getCommandLine() {
-        if(commandline==null)
-            parseCmdLineAndEnvVars();
+        if(commandline == null) {
+            parseCmdLine();
+        }
         return commandline;
     }
 
@@ -127,6 +128,14 @@ public synchronized TreeMap<String,String> getEnvironmentVariables() {
         return envVars;
     }
 
+    private void parseCmdLine() throws WinpException {
+        String s = Native.getCmdLine(pid);
+        if(s == null) {
+            throw new WinpException("Failed to obtain command line for PID = " + pid); 
+        }
+        commandline = s;
+    }
+
     private void parseCmdLineAndEnvVars() {
         String s = Native.getCmdLineAndEnvVars(pid);
         if(s==null)

src/test/java/org/jvnet/winp/NativeAPITest.java

@@ -61,6 +61,8 @@ public void testCriticalProcess() {
     public void testGetCommandLine() {
         int failed = 0;
         int total = 0;
+
+        // Generally this test is unreliable, 299 does not always happen even on Vista+ platforms
         for (WinProcess p : WinProcess.all()) {
             if (p.getPid() < 10) {
                 continue;
@@ -80,12 +82,14 @@ public void testGetCommandLine() {
                     // On Vista and higher, the most common error here is 299, ERROR_PARTIAL_COPY. A bit of
                     // research online seems to suggest that's related to how those versions of Windows use
                     // randomized memory locations for process's
-                    System.out.println("Unexpected failure getting command line for process " + p.getPid() +
+                    Assert.fail("Unexpected failure getting command line for process " + p.getPid() +
                             ": (" + e.getWin32ErrorCode() + ") " + e.getMessage());
                 }
             }
         }
-        System.out.println("Failed to get command lines for " + failed + " of " + total + " processes");
+        if (failed != 0) {
+            System.out.println("Failed to get command lines for " + failed + " of " + total + " processes");
+        }
     }
 
     @Test(expected = WinpException.class)