Add sendCtrlC method

[stephanreiter]

Hi! I'd like to discuss adding a sendCtrlC method to a WinProcess, which I'd need to address a defect in Jenkins (JENKINS-17116) in pull request jenkinsci/jenkins#3414

Sending Ctrl+C is super awkward on Windows. Essentially we have to attach to the target's console, generate the Ctrl+C event, and detach again. That is only safe if we do it in a separate process that has only this purpose.

In the code below I am adding a function that can be called by rundll32 to winp.dll, which does exactly that.
There's a downside to this though: rundll32 is seen as suspicous by many anti-virus solutions, so the invocation might be blocked. What's even worse is that in this case rundll32 might show a message box that access was blocked, which is not acceptable.

The only solution is to not bake the Ctrl+C sending code into winp.dll, but into a separate exe.
Can we ship that exe with the jar and make the DLL call it? The DLL manages to find itself in the filesystem, so if our new exe was placed next to it, we could determine its path as well.

Thanks for taking a look!

[stephanreiter]

Added a commit that moves the ctrl+C sending into a separate executable. I like this better.
Will now added a method to check whether a process exists and then change the test to not wait 4secs, but only until the process it sent ctrl+c to is gone ...

[stephanreiter]

Done. Please review.
Again, the thing I am not sure about is shipping the .exe alongside the .dll. :-/

[oleg-nenashev]

Taking the justification, the approach looks reasonable to me.
OTOH, from what I see, the EXE file still needs to be unpacked to the destination location: https://github.com/kohsuke/winp/blob/b1a6dfbb7ee74fd67456d2230c2728d41eed8ef6/src/main/java/org/jvnet/winp/Native.java#L93-L153

Otherwise it will unlikely work in Jenkins, but I have not checked it so far

[stephanreiter]

Yes! The EXE file needs to live next to the DLL file. So, when we unpack the DLL, we should unpack the EXE as well.

[oleg-nenashev]

requesting changes according to the discussion above

[stephanreiter]

I finally got around to working on this today. Please take a look. I'll need to do more testing.

[stephanreiter]

Can remove code for GetDllFilename again

[stephanreiter]

Done with testing. DLL and EXE are extracted alright and the sendCtrlC() method works.

[oleg-nenashev]

I did some local testing yesterday. Yes, it seems to work well

[stephanreiter]

Thanks for your help!

[stephanreiter]

@oleg-nenashev do you have a timeline for the release of the next version of WinP and can this pull request be included? I'd like to move forward with the change to Jenkins and need a new WinP for that. I appreciate your help.
Thanks, Stephan

[oleg-nenashev]

Hi @stephanreiter . I didn't forget about it. I was doing some internal review of the EXE introduction risks. If it helps, I am happy to ship an alpha version so that I unblock your follow-up PR

[stephanreiter]

That'd be nice, Oleg, but I don't want to generate extra work for you. I am glad to hear that you're making sure this change doesn't break anything in WinP and I'll be happy to wait for a release when the time is here. I was just wondering when that might be. Planning summer activities at work ... :)

[oleg-nenashev]

I have tested the build locally in a cycle, and it seems the test is flaky.
In maybe 20% of tests wp.sendCtrlC(); returns false. Will check whether I can make the test more reliable

Tests run: 8, Failures: 1, Errors: 0, Skipped: 1, Time elapsed: 24.098 sec <<< FAILURE! - in org.jvnet.winp.NativeAPITest
testSendCtrlC(org.jvnet.winp.NativeAPITest)  Time elapsed: 9.698 sec  <<< FAILURE!
java.lang.AssertionError
        at org.junit.Assert.fail(Assert.java:86)
        at org.junit.Assert.assertTrue(Assert.java:41)
        at org.junit.Assert.assertTrue(Assert.java:52)
        at org.jvnet.winp.NativeAPITest.testSendCtrlC(NativeAPITest.java:145)
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.lang.reflect.Method.invoke(Method.java:498)
        at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
        at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
        at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
        at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)

[oleg-nenashev]

So it fails to attach to the console sometimes. I will improve the diagnostics messaging