Subject: Re: bug#69083: Emacs's keyboard hook state is not reset on session lock (Windows)

From: Raffael Stocker <[email-masked]>
Date: Mon, 04 Mar 2024 19:10:33 +0100

Eli Zaretskii <[email-masked]> writes:

>> The order is not important, I just didn't know where to look to get
>> a frame; sorry for the noise.  I now use ‘Fnext_frame’.
>
> Unfortunately, I don't think you cannot use Fnext_frame here.  The
> function which calls it, w32_wnd_proc, runs in a separate thread, so
> it generally cannot access Lisp objects safely.  However, it is okay
> to traverse the list of the frames, as w32_window_to_frame does, see
> the comment at the beginning of w32_wnd_proc.  So I think you could
> use a similar loop with FOR_EACH_FRAME, and use some frame from there,
> perhaps the first one?
>
> Alternatively, and maybe more safely, you could call
> maybe_pass_notification from w32_destroy_window, which is called from
> the main (a.k.a. "Lisp") thread, so then you can use Fnext_frame
> (actually, I would make next_frame extern instead of static and call
> it directly).  This means the notifications are passed a bit before
> the frame is actually deleted by the OS, but I think this is okay?

I went with the first option, using FOR_EACH_FRAME, because I am not
sure about the safety of modifying the kbdhook struct from the other
thread.

Regards,
Raffael

>From 93e6446d3e75b990230618f9d3331fd5a0818d50 Mon Sep 17 00:00:00 2001
From: Raffael Stocker <[email-masked]>
Date: Mon, 4 Mar 2024 19:06:07 +0100
Subject: [PATCH] Fix resetting keyboard hook state on Windows

Register session notifications so Emacs is notified when the
computer is being locked, as required to reset the low level
keyboard hook state (Bug#69083).

* src/w32term.h:
* src/w32fns.c (setup_w32_kbdhook, remove_w32_kbdhook)
(w32_wnd_proc, globals_of_w32fns, maybe_pass_notification):
Register and manage session notifications in GUI Emacs.
* src/w32console.c (initialize_w32_display, find_ime_window):
* src/w32xfns.c (drain_message_queue):
Register notifications and reset keyboard hook state in console
Emacs.
* src/w32.c (term_ntproc): Un-register session notifications
when terminating.
---
 src/w32.c        |  5 +++
 src/w32console.c | 25 ++++++++++++--
 src/w32fns.c     | 84 +++++++++++++++++++++++++++++++++++++++++++++---
 src/w32term.h    |  3 +-
 src/w32xfns.c    | 12 +++++--
 5 files changed, 120 insertions(+), 9 deletions(-)

diff --git a/src/w32.c b/src/w32.c
index df5465c2135..d34ab70f82d 100644
--- a/src/w32.c
+++ b/src/w32.c
@@ -10392,11 +10392,16 @@ check_windows_init_file (void)
     }
 }
 
+/* from w32fns.c */
+extern void remove_w32_kbdhook (void);
+
 void
 term_ntproc (int ignored)
 {
   (void)ignored;
 
+  remove_w32_kbdhook ();
+
   term_timers ();
 
   /* shutdown the socket interface if necessary */
diff --git a/src/w32console.c b/src/w32console.c
index 0936b5f37e6..7dcbc795cac 100644
--- a/src/w32console.c
+++ b/src/w32console.c
@@ -659,6 +659,24 @@ w32_face_attributes (struct frame *f, int face_id)
   return char_attr;
 }
 
+/* The IME window is needed to receive the session notifications
+   required to reset the low level keyboard hook state.  */
+
+static BOOL CALLBACK
+find_ime_window (HWND hwnd, LPARAM arg)
+{
+  char window_class[32];
+
+  GetClassName (hwnd, window_class, sizeof (window_class));
+  if (strcmp (window_class, "IME") == 0)
+    {
+      *(HWND *) arg = hwnd;
+      return FALSE;
+    }
+  /* keep looking */
+  return TRUE;
+}
+
 void
 initialize_w32_display (struct terminal *term, int *width, int *height)
 {
@@ -818,11 +836,14 @@ initialize_w32_display (struct terminal *term, int *width, int *height)
   else
     w32_console_unicode_input = 0;
 
-  /* Setup w32_display_info structure for this frame. */
+  /* Setup w32_display_info structure for this frame.  */
   w32_initialize_display_info (build_string ("Console"));
 
+  HWND hwnd = NULL;
+  EnumThreadWindows (GetCurrentThreadId (), find_ime_window, (LPARAM) &hwnd);
+
   /* Set up the keyboard hook.  */
-  setup_w32_kbdhook ();
+  setup_w32_kbdhook (hwnd);
 }

diff --git a/src/w32fns.c b/src/w32fns.c
index 8d4bd00b91c..3e4a8c475b7 100644
--- a/src/w32fns.c
+++ b/src/w32fns.c
@@ -49,6 +49,7 @@ #define _WIN32_WINNT 0x0600
 #ifdef WINDOWSNT
 #include <mbstring.h>
 #include <mbctype.h>	/* for _getmbcp */
+#include <wtsapi32.h>	/* for WTS(Un)RegisterSessionNotification */
 #endif /* WINDOWSNT */
 
 #if CYGWIN
@@ -204,6 +205,10 @@ DECLARE_HANDLE(HMONITOR);
 typedef HRESULT (WINAPI * DwmSetWindowAttribute_Proc)
   (HWND hwnd, DWORD dwAttribute, IN LPCVOID pvAttribute, DWORD cbAttribute);
 
+typedef BOOL (WINAPI * WTSRegisterSessionNotification_Proc)
+  (HWND hwnd, DWORD dwFlags);
+typedef BOOL (WINAPI * WTSUnRegisterSessionNotification_Proc) (HWND hwnd);
+
 TrackMouseEvent_Proc track_mouse_event_fn = NULL;
 ImmGetCompositionString_Proc get_composition_string_fn = NULL;
 ImmGetContext_Proc get_ime_context_fn = NULL;
@@ -220,6 +225,8 @@ DECLARE_HANDLE(HMONITOR);
 SetThreadDescription_Proc set_thread_description = NULL;
 SetWindowTheme_Proc SetWindowTheme_fn = NULL;
 DwmSetWindowAttribute_Proc DwmSetWindowAttribute_fn = NULL;
+WTSUnRegisterSessionNotification_Proc WTSUnRegisterSessionNotification_fn = NULL;
+WTSRegisterSessionNotification_Proc WTSRegisterSessionNotification_fn = NULL;
 
 extern AppendMenuW_Proc unicode_append_menu;
 
@@ -307,6 +314,7 @@ #define WS_EX_NOACTIVATE 0x08000000L
   int hook_count; /* counter, if several windows are created */
   HHOOK hook;     /* hook handle */
   HWND console;   /* console window handle */
+  HWND notified_wnd; /* window that receives session notifications */
 
   int lwindown;      /* Left Windows key currently pressed (and hooked) */
   int rwindown;      /* Right Windows key currently pressed (and hooked) */
@@ -2744,7 +2752,7 @@ funhook (int code, WPARAM w, LPARAM l)
 /* Set up the hook; can be called several times, with matching
    remove_w32_kbdhook calls.  */
 void
-setup_w32_kbdhook (void)
+setup_w32_kbdhook (HWND hwnd)
 {
   kbdhook.hook_count++;
 
@@ -2800,6 +2808,15 @@ setup_w32_kbdhook (void)
       /* Set the hook.  */
       kbdhook.hook = SetWindowsHookEx (WH_KEYBOARD_LL, funhook,
 				       GetModuleHandle (NULL), 0);
+
+      /* Register session notifications so we get notified about the
+	 computer being locked.  */
+      kbdhook.notified_wnd = NULL;
+      if (hwnd != NULL && WTSRegisterSessionNotification_fn != NULL)
+	{
+	  WTSRegisterSessionNotification_fn (hwnd, NOTIFY_FOR_THIS_SESSION);
+	  kbdhook.notified_wnd = hwnd;
+	}
     }
 }
 
@@ -2811,7 +2828,11 @@ remove_w32_kbdhook (void)
   if (kbdhook.hook_count == 0 && w32_kbdhook_active)
     {
       UnhookWindowsHookEx (kbdhook.hook);
+      if (kbdhook.notified_wnd != NULL
+	  && WTSUnRegisterSessionNotification_fn != NULL)
+	  WTSUnRegisterSessionNotification_fn (kbdhook.notified_wnd);
       kbdhook.hook = NULL;
+      kbdhook.notified_wnd = NULL;
     }
 }
 #endif	/* WINDOWSNT */
@@ -2884,13 +2905,12 @@ check_w32_winkey_state (int vkey)
     }
   return 0;
 }
-#endif	/* WINDOWSNT */
 
 /* Reset the keyboard hook state.  Locking the workstation with Win-L
    leaves the Win key(s) "down" from the hook's point of view - the
    keyup event is never seen.  Thus, this function must be called when
    the system is locked.  */
-static void
+void
 reset_w32_kbdhook_state (void)
 {
   kbdhook.lwindown = 0;
@@ -2900,6 +2920,7 @@ reset_w32_kbdhook_state (void)
   kbdhook.suppress_lone = 0;
   kbdhook.winseen = 0;
 }
+#endif	/* WINDOWSNT */
 
 /* GetKeyState and MapVirtualKey on Windows 95 do not actually distinguish
    between left and right keys as advertised.  We test for this
@@ -4129,6 +4150,47 @@ #define S_TYPES_TO_REPORT_CHARACTER_PAYLOAD_WITH_MODIFIERS ""
   return 0;
 }
 
+/* Maybe pass session notification registration to another frame.  If
+   the frame with window handle HWND is deleted, we must pass the
+   notifications to some other frame, if they have been sent to this
+   frame before and have not already been passed on.  If there is no
+   other frame, do nothing.  */
+
+#ifdef WINDOWSNT
+static void
+maybe_pass_notification (HWND hwnd)
+{
+  if (hwnd == kbdhook.notified_wnd
+      && kbdhook.hook_count > 0 && w32_kbdhook_active)
+    {
+      Lisp_Object tail, frame;
+      struct frame *f;
+      bool found_frame = false;
+
+      FOR_EACH_FRAME (tail, frame)
+	{
+	  f = XFRAME (frame);
+	  if (FRAME_W32_P (f) && FRAME_OUTPUT_DATA (f) != NULL
+	      && FRAME_W32_WINDOW (f) != hwnd)
+	    {
+	      found_frame = true;
+	      break;
+	    }
+	}
+
+      if (found_frame && WTSUnRegisterSessionNotification_fn != NULL
+	  && WTSRegisterSessionNotification_fn != NULL)
+	{
+	  /* There is another frame, pass on the session notification.  */
+	  HWND next_wnd = FRAME_W32_WINDOW (f);
+	  WTSUnRegisterSessionNotification_fn (hwnd);
+	  WTSRegisterSessionNotification_fn (next_wnd, NOTIFY_FOR_THIS_SESSION);
+	  kbdhook.notified_wnd = next_wnd;
+	}
+    }
+}
+#endif	/* WINDOWSNT */
+
 /* Main window procedure */
 
 static LRESULT CALLBACK
@@ -5301,23 +5363,29 @@ w32_wnd_proc (HWND hwnd, UINT msg, WPARAM wParam, LPARAM lParam)
 
 #ifdef WINDOWSNT
     case WM_CREATE:
-      setup_w32_kbdhook ();
+      setup_w32_kbdhook (hwnd);
       goto dflt;
 #endif
 
     case WM_DESTROY:
 #ifdef WINDOWSNT
+      maybe_pass_notification (hwnd);
       remove_w32_kbdhook ();
 #endif
       CoUninitialize ();
       return 0;
 
+#ifdef WINDOWSNT
     case WM_WTSSESSION_CHANGE:
       if (wParam == WTS_SESSION_LOCK)
         reset_w32_kbdhook_state ();
       goto dflt;
+#endif
 
     case WM_CLOSE:
+#ifdef WINDOWSNT
+      maybe_pass_notification (hwnd);
+#endif
       wmsg.dwModifiers = w32_get_modifiers ();
       my_post_msg (&wmsg, hwnd, msg, wParam, lParam);
       return 0;
@@ -11335,6 +11403,14 @@ globals_of_w32fns (void)
   set_thread_description = (SetThreadDescription_Proc)
     get_proc_addr (hm_kernel32, "SetThreadDescription");
 
+#ifdef WINDOWSNT
+  HMODULE wtsapi32_lib = LoadLibrary ("wtsapi32.dll");
+  WTSRegisterSessionNotification_fn = (WTSRegisterSessionNotification_Proc)
+    get_proc_addr (wtsapi32_lib, "WTSRegisterSessionNotification");
+  WTSUnRegisterSessionNotification_fn = (WTSUnRegisterSessionNotification_Proc)
+    get_proc_addr (wtsapi32_lib, "WTSUnRegisterSessionNotification");
+#endif
+
   /* Support OS dark mode on Windows 10 version 1809 and higher.
      See `w32_applytheme' which uses appropriate APIs per version of Windows.
      For future wretches who may need to understand Windows build numbers:
diff --git a/src/w32term.h b/src/w32term.h
index 29ace0b2797..3120c8bd71f 100644
--- a/src/w32term.h
+++ b/src/w32term.h
@@ -779,8 +779,9 @@ #define FILE_NOTIFICATIONS_SIZE 16384
 
 #ifdef WINDOWSNT
 /* Keyboard hooks.  */
-extern void setup_w32_kbdhook (void);
+extern void setup_w32_kbdhook (HWND);
 extern void remove_w32_kbdhook (void);
+extern void reset_w32_kbdhook_state (void);
 extern int check_w32_winkey_state (int);
 #define w32_kbdhook_active (os_subtype != OS_SUBTYPE_9X)
 #else
diff --git a/src/w32xfns.c b/src/w32xfns.c
index fa7d5fbdb61..3d7a1514f72 100644
--- a/src/w32xfns.c
+++ b/src/w32xfns.c
@@ -413,8 +413,16 @@ drain_message_queue (void)
 
   while (PeekMessage (&msg, NULL, 0, 0, PM_REMOVE))
     {
-      if (msg.message == WM_EMACS_FILENOTIFY)
-	retval = 1;
+      switch (msg.message)
+	{
+	case WM_WTSSESSION_CHANGE:
+	  if (msg.wParam == WTS_SESSION_LOCK)
+	    reset_w32_kbdhook_state ();
+	  break;
+	case WM_EMACS_FILENOTIFY:
+	  retval = 1;
+	  break;
+	}
       TranslateMessage (&msg);
       DispatchMessage (&msg);
     }
-- 
2.44.0