Adding a ctrl-C handler in windows

Hi,

I plan to add a control-C handler in win32.c. The handler will simply map it to SIGTERM. Is there any particular reason why control-C is not currently handled?

We need this functionality to use nssm properly. When openvpn is started by nssm, the service has no good ways of gracefully terminating the process. "nssm stop service-name" can send control-C but that is not handled by openvpn. So the default handler catches the signal and just kills the process -- thus the MI or the logs get no reports of the exit, no exit-notify to the remote are generated etc..

Hope I'm not missing some obvious ways to correctly terminate the process from nssm.

Thanks,

Selva

Hi,

I forwarded this email to James - he might have a clue.

Samuli

Currently the Windows implementation, when running in console mode, uses function keys to trigger various Unix signals (see win32_signal_get() function in win32.c).

The current code looks like this:

  switch (win32_keyboard_get (ws))
    {
    case 0x3B: /* F1 -> USR1 */
      ret = SIGUSR1;
      break;
    case 0x3C: /* F2 -> USR2 */
      ret = SIGUSR2;
      break;
    case 0x3D: /* F3 -> HUP */
      ret = SIGHUP;
      break;
    case 0x3E: /* F4 -> TERM */
      ret = SIGTERM;
      break;
    }

It's probably okay to just make CTRL-c generate a SIGTERM as F4 is already doing.

James

Hi,

Thanks for the comment.

In the interactive mode, the console is opened with no ENABLE_PROCESSED_INPUT so ctrl-C will be delivered as key-board input and could be handled just like F4.

With nssm, the console is shared with nssm, so ctrl-C is delivered as a signal. I'll send a patch handling both cases.

Selva

The patch is in the next email.

Handling the key-board input is easy, but making the callback interrupt the event loop looks tricky. I am not well versed with the event handling in the code, so opted to generate a key-pressed event. If there is a better way please suggest.

Simply calling throw_signal in the callback will work, but could take several seconds before its noticed.

Tested on windows 7 with cmd-line use and start/stop with nssm. For nssm, the default delay after ctrl-C is 1500 msec which is not enough for the process to exit if exit-notify is being used. About 1 second per extit notify trials + an extra 1 second is suggested.

 src/openvpn/win32.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 54 insertions(+)

2.6.2

Handle ctrl-C or ctrl-Break sent to the console as a SIGTERM. Depending on the console mode, windows delivers ctrl-C as a keyboard input or as a signal. We handle both cases. This allows graceful termination of the openvpn from programs such as nssm.

---
 src/openvpn/win32.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 54 insertions(+)

index 7c89a5a..feae73c 100644
@@ -324,6 +324,55 @@ net_event_win32_close (struct net_event_win32 *ne)
  * (2) Service mode -- map Windows event object to SIGTERM
  */
 
+static bool
+win_keyboard_put_ctrlC (HANDLE stdin_handle)
+{
+  DWORD tmp;
+  INPUT_RECORD ir;
+
+  CLEAR(ir);
+  ir.EventType = KEY_EVENT;
+  ir.Event.KeyEvent.bKeyDown = true;
+  ir.Event.KeyEvent.wRepeatCount = 1;
+  ir.Event.KeyEvent.uChar.AsciiChar = 0x03; /* ctrl-C */
+  if (WriteConsoleInput(stdin_handle, &ir, 1, &tmp))
+     return true;
+  else
+     return false;
+}
+
+/*
+ * Callback to handle console ctrl events
+ */
+static bool
+win_ctrl_handler (DWORD signum)
+{
+  msg(D_LOW, "win_ctrl_handler: signal received (code=%d)", signum);
+
+  HANDLE stdin_handle = GetStdHandle (STD_INPUT_HANDLE);
+  if (stdin_handle == INVALID_HANDLE_VALUE)
+  {
+     msg(M_WARN, "Warning: win_ctrl_handler: no console input, signal not handled");
+     return false;
+  }
+
+  switch (signum)
+    {
+    case CTRL_C_EVENT:
+    case CTRL_BREAK_EVENT:
+      if (win_keyboard_put_ctrlC (stdin_handle))
+        return true;
+      else
+        msg(M_NONFATAL, "win_ctrl_handler: write to console input failed");
+      break;
+    default:
+      msg(D_LOW, "win_ctrl_handler: unknown signal (code=%d) not handled", signum);
+      break;
+    }
+  /* pass all other signals to the next handler */
+  return false;
+}
+
 void
 win32_signal_clear (struct win32_signal *ws)
 {

@@ -369,6 +418,8 @@ win32_signal_open (struct win32_signal *ws,
 		  ws->console_mode_save_defined = true;
 		}
 	      ws->mode = WSO_MODE_CONSOLE;
+             if (!SetConsoleCtrlHandler ((PHANDLER_ROUTINE) win_ctrl_handler, true))
+                msg (M_NONFATAL, "Error: win32_signal_open: SetConsoleCtrlHandler failed");
 	    }
 	  else
 	    ws->in.read = INVALID_HANDLE_VALUE; /* probably running as a service */

@@ -512,6 +563,9 @@ win32_signal_get (struct win32_signal *ws)
 	    case 0x3E: /* F4 -> TERM */
 	      ret = SIGTERM;
 	      break;
+           case 0x03: /* CTRL-C -> TERM */
+             ret = SIGTERM;
+             break;
 	    }
 	}
       if (ret)

2.6.2

v2 changes
- cleaner, hopefully easier to get a code review :)
- handles both console mode and service mode
-- >8 --

Handle ctrl-C or ctrl-Break sent to the console as a SIGTERM. Depending on the console mode, windows delivers ctrl-C as a keyboard input or as a signal. We handle both cases. This allows graceful termination of openvpn from programs such as nssm. Works in both console mode and service mode.

---
 src/openvpn/win32.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 52 insertions(+)

index d06b41f..1f9bda0 100644
@@ -324,6 +324,53 @@ net_event_win32_close (struct net_event_win32 *ne)
  * (2) Service mode -- map Windows event object to SIGTERM
  */
 
+static void
+win_trigger_event(struct win32_signal *ws)
+{
+  if (ws->mode == WSO_MODE_SERVICE && HANDLE_DEFINED(ws->in.read))
+    SetEvent (ws->in.read);
+  else /* generate a key-press event */
+    {
+      DWORD tmp;
+      INPUT_RECORD ir;
+      HANDLE stdin_handle = GetStdHandle(STD_INPUT_HANDLE);
+
+      CLEAR(ir);
+      ir.EventType = KEY_EVENT;
+      ir.Event.KeyEvent.bKeyDown = true;
+      if (!stdin_handle || !WriteConsoleInput(stdin_handle, &ir, 1, &tmp))
+        msg(M_WARN|M_ERRNO, "WARN: win_trigger_event: WriteConsoleInput");
+    }
+}
+
+/*
+ * Callback to handle console ctrl events
+ */
+static bool WINAPI
+win_ctrl_handler (DWORD signum)
+{
+  msg(D_LOW, "win_ctrl_handler: signal received (code=%lu)", (unsigned long) signum);
+
+  if (siginfo_static.signal_received == SIGTERM)
+     return true;
+
+  switch (signum)
+    {
+    case CTRL_C_EVENT:
+    case CTRL_BREAK_EVENT:
+      throw_signal(SIGTERM);
+      /* trigget the win32_signal to interrupt the event loop */
+      win_trigger_event(&win32_signal);
+      return true;
+      break;
+    default:
+      msg(D_LOW, "win_ctrl_handler: signal (code=%lu) not handled", (unsigned long) signum);
+      break;
+    }
+  /* pass all other signals to the next handler */
+  return false;
+}
+
 void
 win32_signal_clear (struct win32_signal *ws)
 {

@@ -403,6 +450,9 @@ win32_signal_open (struct win32_signal *ws,
 	    ws->mode = WSO_MODE_SERVICE;
 	}
     }
+    /* set the ctrl handler in both console and service modes */
+    if (!SetConsoleCtrlHandler ((PHANDLER_ROUTINE) win_ctrl_handler, true))
+       msg (M_WARN|M_ERRNO, "WARN: SetConsoleCtrlHandler failed");
 }
 
 static bool

@@ -512,6 +562,9 @@ win32_signal_get (struct win32_signal *ws)
 	    case 0x3E: /* F4 -> TERM */
 	      ret = SIGTERM;
 	      break;
+           case 0x03: /* CTRL-C -> TERM */
+             ret = SIGTERM;
+             break;
 	    }
 	}
       if (ret)

2.6.2

Hi,

If I read this right, the new code uses WriteConsoleInput() only to signal "hey, some work to do here, wake up!" without actually sending any keypress-to-itself, and the signal itself is sent by throw_signal()?

This is certainly nicer :-)

(Is it actually quicker as well, so does the nssm timeout still need to be adjusted? Haven't seen feedback from Samuli here yet)

gert

USENET is *not* the non-clickable part of WWW!
                                                           //www.muc.de/~gert/
Gert Doering - Munich, Germany                             [email protected]...
fax: +49-89-35655025                        [email protected]...

Hi,

Yes, that's the idea.

This is certainly nicer :-)

Agree :)

In my tests openvpn exit processing takes only a few hundred msec, but if exit-notify is enabled that adds about 1 to 2 seconds[*]. As that option could be pushed as well its safer to setup nssm with a delay of 2500 msec between ctrl-C and 'kill without prejudice' (aka Terminate).

Thanks,

Selva

[*} actually with exit-notify n, it should only take n-1 seconds to send n notifies, but integer arithmetic makes it always a little more than n-1. And for some reason it appears to wait a second even if another notify doesn't have to be sent. I've not looked into that part of the code carefully...

Hi,

Ah, yes. Thanks :-) (you and Samuli need to work on the NSSM setup stuff then, I don't have an eye on the windows stuff, so won't be able to track this so it's not getting lost)

Our timer granularity is "1 second" for most stuff, and we tend to add another second "just to be sure"... ran into this when trying to speed up client PUSH_REQUEST sending in commit afb93fac803f

gert

USENET is *not* the non-clickable part of WWW!
                                                           //www.muc.de/~gert/
Gert Doering - Munich, Germany                             [email protected]...
fax: +49-89-35655025                        [email protected]...

Hi,

For now we only need to add it in the wiki entry on nssm -- I'll try to edit it or request Samuli. Then if/when we replace the 'aService' with nssm or start shipping an nssm script, it could be taken from the wiki example.

Well, that explains it. Thanks.

Selva

ACK, on the basis that the code looks reasonable, solves a real problem on Win32 and does not affect other platforms. I have *not* tested it

Your patch has been applied to the master and release/2.3 branch.

commit 87f1be66e88303c51520925f169dc5a8aa58a7f2 (master) commit 29f883fb31b362ffa21f11b672f3f4ea638c360e (release/2.3)

Author: Selva Nair
Date:   Mon Nov 16 21:48:09 2015 -0500

     Handle ctrl-C and ctrl-break events on Windows

     Signed-off-by: Selva Nair <[email protected]...>
     Acked-by: Gert Doering <[email protected]...>
     Message-Id: <[email protected]...>
     URL: http://article.gmane.org/gmane.network.openvpn.devel/10513
     Signed-off-by: Gert Doering <[email protected]...>

--
kind regards,

Gert Doering

Hi,

Arne might have an idea here... I do not understand the event handling that well either.

[..]

So this would be something for Samuli to adjust when setting up NSSM? Like, 2500 ms?

gert

USENET is *not* the non-clickable part of WWW!
                                                           //www.muc.de/~gert/
Gert Doering - Munich, Germany                             [email protected]...
fax: +49-89-35655025                        [email protected]...

Hi,

That is correct. It could be added to the nssm info page that Samuli maintains. I use 3000 msec with "--explicit-exit-notify 2" and works well.

Selva