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

From: Raffael Stocker <[email-masked]>
Date: Mon, 26 Feb 2024 21:50:43 +0100

Eli Zaretskii <[email-masked]> writes:

>> Is there a better place where the remove_w32_kbdhook call could go such
>> that cleanup can always happen?
>
> I think that place is term_ntproc.

I have added a call to ‘remove_w32_kbdhook’ there, but also left it with
the WM_DESTROY message.  The cleanup now always happens and the
setup/remove calls are correctly nested when using multiple frames.

>> >> Unfortunately, this didn't work for me.  I tried calling
>> >> ‘EnumWindows(find_child_console, ...)’ with a ‘child_process’ instance
>> >> containing the current process id as returned by ‘GetCurrentProcessId’,
>> >> but I don't seem to get a useful window handle.
>> >
>> > What do you mean?  What is the result of using find_child_console?
>> > does the condition in find_child_console, which looks at the
>> > process_id of all windows, never match the process ID of the Emacs
>> > session running with -nw?  Or what else goes wrong?

I figured this out now.  The ‘find_child_console’ function looks for a
‘ConsoleWindowClass’ window, but the Emacs process itself only has
‘Emacs Clipboard’ and ‘IME’ windows.  The latter seems to be the one I
need.  I added a function ‘find_ime_window’ in ‘w32console.c’ that
checks for this window when called through ‘EnumThreadWindows’.  I can
now register for session notifications using this window.

To handle the notifications, I added some code to ‘drain_message_queue’,
calling ‘reset_w32_kbdhook_state’ from there.  This now correctly resets
the hook state in console Emacs.

I added ‘WINDOWSNT’ #ifdefs around calls to and the definition of
‘reset_w32_kbdhook_state’, as with the setup/remove functions.  I also
cleaned up the ‘remove_w32_kbdhook’ function to use the previously
obtained function pointer.

I believe the attached version should now tick all the important boxes.

Regards,
Raffael

diff --git a/src/w32.c b/src/w32.c
index df5465c2135..6744b45bde7 100644
--- a/src/w32.c
+++ b/src/w32.c
@@ -10392,11 +10392,17 @@ 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..bcee9e18fe6 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)
 {
@@ -821,8 +839,11 @@ initialize_w32_display (struct terminal *term, int *width, int *height)
   /* Setup w32_display_info structure for this frame. */
   w32_initialize_display_info (build_string ("Console"));
 
-  /* Set up the keyboard hook.  */
-  setup_w32_kbdhook ();
+  HWND hwnd = NULL;
+  EnumThreadWindows (GetCurrentThreadId (), find_ime_window, (LPARAM) &hwnd);
+
+  /* Set up the keyboard hook. */
+  setup_w32_kbdhook (hwnd);
 }

diff --git a/src/w32fns.c b/src/w32fns.c
index 8d4bd00b91c..168763f7215 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,7 @@ DECLARE_HANDLE(HMONITOR);
 SetThreadDescription_Proc set_thread_description = NULL;
 SetWindowTheme_Proc SetWindowTheme_fn = NULL;
 DwmSetWindowAttribute_Proc DwmSetWindowAttribute_fn = NULL;
+WTSUnRegisterSessionNotification_Proc WTSUnRegisterSessionNotification_fn = NULL;
 
 extern AppendMenuW_Proc unicode_append_menu;
 
@@ -307,6 +313,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 window;    /* window handle used for session notification */
 
   int lwindown;      /* Left Windows key currently pressed (and hooked) */
   int rwindown;      /* Right Windows key currently pressed (and hooked) */
@@ -2744,7 +2751,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 +2807,21 @@ 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. */
+      if (hwnd != NULL)
+	{
+	  kbdhook.window = hwnd;
+	  HMODULE wtsapi32_lib = LoadLibrary ("wtsapi32.dll");
+	  WTSRegisterSessionNotification_Proc WTSRegisterSessionNotification_fn
+	    = (WTSRegisterSessionNotification_Proc)
+	    get_proc_addr (wtsapi32_lib, "WTSRegisterSessionNotification");
+	  WTSUnRegisterSessionNotification_fn = (WTSUnRegisterSessionNotification_Proc)
+	    get_proc_addr (wtsapi32_lib, "WTSUnRegisterSessionNotification");
+	  if (WTSRegisterSessionNotification_fn != NULL)
+	    WTSRegisterSessionNotification_fn (hwnd, NOTIFY_FOR_THIS_SESSION);
+	}
     }
 }
 
@@ -2811,6 +2833,9 @@ remove_w32_kbdhook (void)
   if (kbdhook.hook_count == 0 && w32_kbdhook_active)
     {
       UnhookWindowsHookEx (kbdhook.hook);
+      if (kbdhook.window != NULL
+	  && WTSUnRegisterSessionNotification_fn != NULL)
+	WTSUnRegisterSessionNotification_fn (kbdhook.window);
       kbdhook.hook = NULL;
     }
 }
@@ -2884,13 +2909,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 +2924,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
@@ -5301,7 +5326,7 @@ 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
 
@@ -5312,10 +5337,12 @@ w32_wnd_proc (HWND hwnd, UINT msg, WPARAM wParam, LPARAM lParam)
       CoUninitialize ();
       return 0;
 
+#ifdef WINDOWSNT
     case WM_WTSSESSION_CHANGE:
       if (wParam == WTS_SESSION_LOCK)
         reset_w32_kbdhook_state ();
       goto dflt;
+#endif
 
     case WM_CLOSE:
       wmsg.dwModifiers = w32_get_modifiers ();
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);
     }