Fix signal handling on Windows

- In win32_signal_get() re-order the check so that Windows
  signals are picked up even if signal_received is non-zero

- When management is not active, management_sleep() becomes sleep()
  but it is not interruptible by signals on Windows. Fix this by
  periodically checking for signal.

Trac: #311 #639 (windows specific part)
Github: Fixes OpenVPN/openvpn#205 (windows specific part)

Note: if stuck in address resolution, press ctrl-C and wait for
getaddrinfo() to timeout.

v2: WIN32 --> _WIN32
    add a chunk in management_sleep that was missed by sloppy
    conflict-resolution

v3: following review by Lev Stipakov <lstipakov@gmail.com>
  win32_sleep()
    - Early fallback to Sleep() if no wait handles -- less indentation
    - Check signal only if wait-object triggered
    - Exit the while loop if not safe to continue
  Behaviour of win32_sleep(0) checking signal is retained though may be
  redundant

v4: Avoid Sleep(0) and never loop back to wait again if wait-failed

Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Lev Stipakov <lstipakov@gmail.com>
Message-Id: <20230106005438.1664046-1-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25895.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit 22977577ed128ac953e7ebfe30f839bcf651b334)

diff --git a/src/openvpn/manage.c b/src/openvpn/manage.c
index 7e326702f5c12cb231ed82cc4ebc7a98b7679cea..d2d2ca9adfe74ead2958d2b248669bf2e0ed6eb5 100644 (file)
--- a/src/openvpn/manage.c
+++ b/src/openvpn/manage.c
@@ -4117,9 +4117,16 @@ management_sleep(const int n)
     {
         management_event_loop_n_seconds(management, n);
     }
-    else if (n > 0)
+    else
     {
-        sleep(n);
+#ifdef _WIN32
+        win32_sleep(n);
+#else
+        if (n > 0)
+        {
+            sleep(n);
+        }
+#endif
     }
 }
 
@@ -4160,13 +4167,18 @@ man_persist_client_stats(struct management *man, struct context *c)
 
 #else  /* ifdef ENABLE_MANAGEMENT */
 
+#include "win32.h"
 void
 management_sleep(const int n)
 {
+#ifdef _WIN32
+    win32_sleep(n);
+#else
     if (n > 0)
     {
         sleep(n);
     }
+#endif /* ifdef _WIN32 */
 }
 
 #endif /* ENABLE_MANAGEMENT */

diff --git a/src/openvpn/win32.c b/src/openvpn/win32.c
index c3520bca00b45dfaa16b1257d82e301454ab3bdc..569e086ea1681cbf69622d99c2ee2cc7df9807cd 100644 (file)
--- a/src/openvpn/win32.c
+++ b/src/openvpn/win32.c
@@ -642,50 +642,44 @@ int
 win32_signal_get(struct win32_signal *ws)
 {
     int ret = 0;
-    if (siginfo_static.signal_received)
-    {
-        ret = siginfo_static.signal_received;
-    }
-    else
+
+    if (ws->mode == WSO_MODE_SERVICE)
     {
-        if (ws->mode == WSO_MODE_SERVICE)
+        if (win32_service_interrupt(ws))
         {
-            if (win32_service_interrupt(ws))
-            {
-                ret = SIGTERM;
-            }
+            ret = SIGTERM;
         }
-        else if (ws->mode == WSO_MODE_CONSOLE)
+    }
+    else if (ws->mode == WSO_MODE_CONSOLE)
+    {
+        switch (win32_keyboard_get(ws))
         {
-            switch (win32_keyboard_get(ws))
-            {
-                case 0x3B: /* F1 -> USR1 */
-                    ret = SIGUSR1;
-                    break;
+            case 0x3B: /* F1 -> USR1 */
+                ret = SIGUSR1;
+                break;
 
-                case 0x3C: /* F2 -> USR2 */
-                    ret = SIGUSR2;
-                    break;
+            case 0x3C: /* F2 -> USR2 */
+                ret = SIGUSR2;
+                break;
 
-                case 0x3D: /* F3 -> HUP */
-                    ret = SIGHUP;
-                    break;
+            case 0x3D: /* F3 -> HUP */
+                ret = SIGHUP;
+                break;
 
-                case 0x3E: /* F4 -> TERM */
-                    ret = SIGTERM;
-                    break;
+            case 0x3E: /* F4 -> TERM */
+                ret = SIGTERM;
+                break;
 
-                case 0x03: /* CTRL-C -> TERM */
-                    ret = SIGTERM;
-                    break;
-            }
-        }
-        if (ret)
-        {
-            throw_signal(ret); /* this will update signinfo_static.signal received */
+            case 0x03: /* CTRL-C -> TERM */
+                ret = SIGTERM;
+                break;
         }
     }
-    return ret;
+    if (ret)
+    {
+        throw_signal(ret); /* this will update signinfo_static.signal received */
+    }
+    return (siginfo_static.signal_received);
 }
 
 void
@@ -1603,4 +1597,47 @@ set_openssl_env_vars()
     }
 }
 
+void
+win32_sleep(const int n)
+{
+    if (n < 0)
+    {
+        return;
+    }
+
+    /* Sleep() is not interruptible. Use a WAIT_OBJECT to catch signal */
+
+    if (!HANDLE_DEFINED(win32_signal.in.read))
+    {
+        if (n > 0)
+        {
+            Sleep(n*1000);
+        }
+        return;
+    }
+
+    update_time();
+    time_t expire = now + n;
+
+    while (expire >= now)
+    {
+        DWORD status = WaitForSingleObject(win32_signal.in.read, (expire-now)*1000);
+        if ((status == WAIT_OBJECT_0 && win32_signal_get(&win32_signal))
+            || status == WAIT_TIMEOUT)
+        {
+            return;
+        }
+
+        update_time();
+
+        if (status != WAIT_OBJECT_0) /* wait failed or some unexpected error ? */
+        {
+            if (expire > now)
+            {
+                Sleep((expire-now)*1000);
+            }
+            return;
+        }
+    }
+}
 #endif /* ifdef _WIN32 */

diff --git a/src/openvpn/win32.h b/src/openvpn/win32.h
index b1371999ba4990b6e0bc4a20f14e689bd08bf62f..26d5ef8fff9b364cb7fa0b9de9cdeb765a3ea165 100644 (file)
--- a/src/openvpn/win32.h
+++ b/src/openvpn/win32.h
@@ -330,5 +330,8 @@ openvpn_execve(const struct argv *a, const struct env_set *es, const unsigned in
 bool
 openvpn_swprintf(wchar_t *const str, const size_t size, const wchar_t *const format, ...);
 
+/* Sleep that can be interrupted by signals and exit event */
+void win32_sleep(const int n);
+
 #endif /* ifndef OPENVPN_WIN32_H */
 #endif /* ifdef _WIN32 */