diff --git a/native/java-interface.cpp b/native/java-interface.cpp index 7e72cdc..bbb369c 100644 --- a/native/java-interface.cpp +++ b/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) { diff --git a/native/sendctrlc/main.cpp b/native/sendctrlc/main.cpp index 964f82e..f6603c9 100644 --- a/native/sendctrlc/main.cpp +++ b/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); diff --git a/native/winp.cpp b/native/winp.cpp index bec3b67..a6eab7a 100644 --- a/native/winp.cpp +++ b/native/winp.cpp @@ -9,6 +9,10 @@ #include #include +// 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(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(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(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); diff --git a/native/winp.h b/native/winp.h index 1bd5e80..873fc66 100644 --- a/native/winp.h +++ b/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 diff --git a/src/main/java/org/jvnet/winp/Native.java b/src/main/java/org/jvnet/winp/Native.java index 4d696cc..2f6c57a 100755 --- a/src/main/java/org/jvnet/winp/Native.java +++ b/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); diff --git a/src/main/java/org/jvnet/winp/WinProcess.java b/src/main/java/org/jvnet/winp/WinProcess.java index 0f0c08f..5e6db2a 100755 --- a/src/main/java/org/jvnet/winp/WinProcess.java +++ b/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); diff --git a/src/test/java/org/jvnet/winp/NativeAPITest.java b/src/test/java/org/jvnet/winp/NativeAPITest.java index ab95225..d785d1d 100644 --- a/src/test/java/org/jvnet/winp/NativeAPITest.java +++ b/src/test/java/org/jvnet/winp/NativeAPITest.java @@ -135,14 +135,14 @@ 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; @@ -150,10 +150,19 @@ public void testSendCtrlC() throws Exception { 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;