Improve diagnostics and error propagation of CtrlC commands

native/java-interface.cpp

@@ -9,7 +9,7 @@ JNIEXPORT jboolean JNICALL Java_org_jvnet_winp_Native_kill(JNIEnv* env, jclass c
 
 JNIEXPORT jboolean JNICALL Java_org_jvnet_winp_Native_sendCtrlC(JNIEnv* env, jclass clazz, jint pid, jstring sendctrlcExePath) {
   const wchar_t* exePath = (wchar_t*)env->GetStringChars(sendctrlcExePath, NULL);
-  return SendCtrlC(pid, exePath);
+  return SendCtrlC(env, clazz, pid, exePath);
 }
 
 JNIEXPORT jint JNICALL Java_org_jvnet_winp_Native_setPriority(JNIEnv* env, jclass clazz, jint pid, jint priority) {

native/sendctrlc/main.cpp

@@ -15,7 +15,7 @@ int main(int argc, char** argv) {
 
   FreeConsole();
   if (!AttachConsole(pid)) {
-    return 1;
+    return GetLastError();
   }
 
   SetConsoleCtrlHandler(NULL, TRUE);

native/winp.cpp

@@ -9,6 +9,10 @@
 #include <string>
 #include <vector>
 
+// Max size of error messages in the methods. Defines static buffer sizes
+// This file uses a long buffer, because it prints executable paths in some cases.
+#define ERRMSG_SIZE 512
+
 //---------------------------------------------------------------------------
 // SendCtrlC
 //
@@ -19,8 +23,10 @@
 //
 //  Returns:
 //	  TRUE, if successful, FALSE - otherwise.
+//    When used from JNI, exceptions may be thrown instead
 //
-BOOL WINAPI SendCtrlC(IN DWORD dwProcessId, const wchar_t* pExePath) {
+BOOL WINAPI SendCtrlC(JNIEnv* pEnv, jclass clazz, IN DWORD dwProcessId, const wchar_t* pExePath) {
+  char errorBuffer[ERRMSG_SIZE];
   STARTUPINFO         si;
   PROCESS_INFORMATION pi;
   ZeroMemory(&si, sizeof(si));
@@ -37,13 +43,25 @@ BOOL WINAPI SendCtrlC(IN DWORD dwProcessId, const wchar_t* pExePath) {
   BOOL success = FALSE;
   if (started) {
     // wait for termination if the process started, max. 5 secs
-    WaitForSingleObject(pi.hProcess, 5000);
+    DWORD ret = WaitForSingleObject(pi.hProcess, 5000);
+    if (ret != WAIT_OBJECT_0) {
+        sprintf_s<ERRMSG_SIZE>(errorBuffer, "Failed to send Ctrl+C to process with pid=%d. WaitForSingleObject exit code: %d (last error: d).", dwProcessId, ret, GetLastError());
+        reportError(pEnv, errorBuffer);
+    }
 
     // then set success flag if the exit code was 0
     DWORD exit_code;
     if (GetExitCodeProcess(pi.hProcess, &exit_code) != FALSE) {
       success = (exit_code == 0);
+      if (exit_code != 0) {
+        sprintf_s<ERRMSG_SIZE>(errorBuffer, "External Ctrl+C execution failed for process pid=%d. Ctrl+C process exited with code %d: %s.", dwProcessId, exit_code,
+            (exit_code == -1) ? "Wrong arguments" : "Failed to attach to the console (see the AttachConsole WinAPI call)");
+        reportError(pEnv, errorBuffer);
+      }
     }
+  } else {
+    sprintf_s<ERRMSG_SIZE>(errorBuffer, "Failed to send Ctrl+C to process with pid=%d. Signal process did not start: %s.", dwProcessId, cmd);
+    reportError(pEnv, errorBuffer);
   }
 
   CloseHandle(pi.hProcess);

native/winp.h

@@ -6,7 +6,7 @@
 #define reportError(env,msg)	error(env,__FILE__,__LINE__,msg);
 void error(JNIEnv* env, const char* file, int line, const char* msg);
 
-BOOL WINAPI SendCtrlC(IN DWORD dwProcessId, const wchar_t* pExePath);
+BOOL WINAPI SendCtrlC(JNIEnv* pEnv, jclass clazz, IN DWORD dwProcessId, const wchar_t* pExePath);
 
 //
 // Kernel32.dll

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

@@ -1,6 +1,8 @@
 package org.jvnet.winp;
 
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
+
+import javax.annotation.CheckReturnValue;
 import java.math.BigInteger;
 import java.net.URL;
 import java.net.URLDecoder;
@@ -70,8 +72,18 @@ class Native {
 
     private static String ctrlCExePath;
 
-    public static boolean sendCtrlC(int pid) {
+    /**
+     * Sends Ctrl+C to the process.
+     * Due to the Windows platform specifics, this execution will spawn a separate thread to deliver the signal.
+     * This process is expected to be executed within a 5-second timeout.
+     * @param pid PID to receive the signal
+     * @return {@code true} if the signal was delivered successfully
+     * @throws WinpException Execution error
+     */
+    @CheckReturnValue
+    public static boolean sendCtrlC(int pid) throws WinpException {
         if (ctrlCExePath == null) {
+            LOGGER.log(Level.WARNING, "Cannot send the CtrlC signal to the process. Cannot find the executable {0}.dll", CTRLCEXE_NAME);
             return false;
         }
         return sendCtrlC(pid, ctrlCExePath);

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

@@ -1,5 +1,6 @@
 package org.jvnet.winp;
 
+import javax.annotation.CheckReturnValue;
 import java.lang.reflect.Field;
 import java.util.Comparator;
 import java.util.TreeMap;
@@ -78,7 +79,15 @@ public void kill() {
         Native.kill(pid,false);
     }
 
-    public boolean sendCtrlC() {
+    /**
+     * Sends Ctrl+C to the process.
+     * Due to the Windows platform specifics, this execution will spawn a separate thread to deliver the signal.
+     * This process is expected to be executed within a 5-second timeout.
+     * @return {@code true} if the signal was delivered successfully
+     * @throws WinpException Execution error
+     */
+    @CheckReturnValue
+    public boolean sendCtrlC() throws WinpException {
         if (LOGGER.isLoggable(FINE))
             LOGGER.fine(String.format("Attempting to send CTRL+C to pid=%d (%s)",pid,getCommandLine()));
         return Native.sendCtrlC(pid);

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

@@ -135,25 +135,34 @@ public void testPingAsDelay() throws Exception {
 
     @Test
     public void testSendCtrlC() throws Exception {
-        Process p = spawnProcess("PING", "-n", "10", "127.0.0.1"); // run for 10 secs
+        Process p = spawnProcess("PING", "-n", "20", "127.0.0.1"); // run for 20 secs
 
         WinProcess wp = new WinProcess(p);
-        assertTrue(wp.isRunning());
+        assertTrue("Process is not running: " + p, wp.isRunning());
 
         // send Ctrl+C, then wait for a max of 4 secs
         boolean sent = wp.sendCtrlC();
-        assertTrue(sent);
+        assertTrue("Failed to send the Ctrl+C signal to the process: " + p, sent);
         for (int i = 0; i < 40; ++i) {
             if (!wp.isRunning()) {
                 break;
             }
             Thread.sleep(100);
         }
 
-        assertTrue(!wp.isRunning());
+        assertTrue("Process has not been terminated after Ctrl+C", !wp.isRunning());
         wp.killRecursively();
     }
 
+    @Test(expected = WinpException.class)
+    public void testSendCtrlC_nonExistentPID() throws Exception {
+        WinProcess wp = new WinProcess(Integer.MAX_VALUE);
+        assertFalse("Process is running when it should not: " + wp, wp.isRunning());
+
+        // send Ctrl+C, then wait for a max of 4 secs
+        boolean sent = wp.sendCtrlC();
+    }
+
     @Test
     public void shouldFailForNonExistentProcess() {
         int nonExistentPid = Integer.MAX_VALUE;