Subject: Emacs's keyboard hook state is not reset on session lock (Windows)
From: Raffael Stocker <[email-masked]>
To: [email-masked]
Date: Mon, 12 Feb 2024 20:04:50 +0100
List-Id: <debbugs-submit.debbugs.gnu.org>

Hi,

when locking the computer using WIN+L on Windows, Emacs is supposed to
reset its keyboard hook state.  This is done in the function
‘reset_w32_kbdhook_state’ in ‘w32fns.c’, which is called upon receiving
the ‘WM_WTSSESSION_CHANGE’ message with the ‘WTS_SESSION_LOCK’
parameter.  However, this message is never received, because to receive
it, an application must register for session notifications, which Emacs
doesn't do.

When Emacs can't reset its state, the handling of the windows key is
confused after locking and unlocking the computer, but the issue seems
to auto-correct when a windows key is pressed again.  I didn't observe
any real problem with it, but I have not configured any non-default
handling of the windows keys in Emacs.  It may be different in those
cases.

Note that while this bug looks like it might be connected to bug#68914,
I believe it's a separate issue.  The behaviour described there is
neither triggered by this bug nor solved by the attached patch.

The attached patch rectifies the situation, but to do so, Emacs must be
linked with ‘wtsapi32.dll’.  I am not sure whether there is a different
way of resetting the state that doesn't require the introduction of a
new dependency.

To receive session notifications, one must provide a window handle,
which is fine if Emacs does not run in console mode.  I don't know
whether it is possible to get these notifications in console Emacs; at
least using the console handle didn't work for me.

I also noticed that while the keyboard hook is set up in console mode
using ‘setup_w32_kbdhook’, there does not seem to be a corresponding
call to ‘remove_w32_kbdhook’.  Also, in console mode the keyboard hook
is always installed, while in GUI Emacs it is only installed when
‘WINDOWSNT’ is defined.

I have zero experience in Windows programming, so it would be highly
desirable if someone with more knowledge could comment on the issue and
my proposed solution.

Regards,
Raffael

diff --git a/configure.ac b/configure.ac
index 847fdbd54d2..415430de417 100644
--- a/configure.ac
+++ b/configure.ac
@@ -3090,6 +3090,7 @@ AC_DEFUN
     fi
     W32_LIBS="$W32_LIBS -lwinmm -lusp10 -lgdi32 -lcomdlg32"
     W32_LIBS="$W32_LIBS -lmpr -lwinspool -lole32 -lcomctl32"
+    W32_LIBS="$W32_LIBS -lwtsapi32"
     W32_RES_LINK="\$(EMACSRES)"
     CLIENTRES="emacsclient.res"
     CLIENTW="emacsclientw\$(EXEEXT)"
diff --git a/src/w32console.c b/src/w32console.c
index 0936b5f37e6..0bd23f06832 100644
--- a/src/w32console.c
+++ b/src/w32console.c
@@ -821,8 +821,12 @@ 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"));
 
+#ifdef WINDOWSNT
   /* Set up the keyboard hook.  */
-  setup_w32_kbdhook ();
+  /* FIXME where can this be cleaned up, that is, where can we call
+     remove_w32_kbdhook? */
+  setup_w32_kbdhook (NULL);
+#endif
 }

diff --git a/src/w32fns.c b/src/w32fns.c
index 8d4bd00b91c..1d5a591466c 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)RegisterSessionNotificationEx */
 #endif /* WINDOWSNT */
 
 #if CYGWIN
@@ -2744,7 +2745,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,17 +2801,22 @@ setup_w32_kbdhook (void)
       /* Set the hook.  */
       kbdhook.hook = SetWindowsHookEx (WH_KEYBOARD_LL, funhook,
 				       GetModuleHandle (NULL), 0);
+      if (hwnd != NULL)
+	WTSRegisterSessionNotificationEx (WTS_CURRENT_SERVER, hwnd,
+					  NOTIFY_FOR_THIS_SESSION);
     }
 }
 
 /* Remove the hook.  */
 void
-remove_w32_kbdhook (void)
+remove_w32_kbdhook (HWND hwnd)
 {
   kbdhook.hook_count--;
   if (kbdhook.hook_count == 0 && w32_kbdhook_active)
     {
       UnhookWindowsHookEx (kbdhook.hook);
+      if (hwnd != NULL)
+	WTSUnRegisterSessionNotificationEx (WTS_CURRENT_SERVER, hwnd);
       kbdhook.hook = NULL;
     }
 }
@@ -5301,13 +5307,13 @@ 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
-      remove_w32_kbdhook ();
+      remove_w32_kbdhook (hwnd);
 #endif
       CoUninitialize ();
       return 0;
diff --git a/src/w32term.h b/src/w32term.h
index 29ace0b2797..374c8055ed8 100644
--- a/src/w32term.h
+++ b/src/w32term.h
@@ -779,8 +779,8 @@ #define FILE_NOTIFICATIONS_SIZE 16384
 
 #ifdef WINDOWSNT
 /* Keyboard hooks.  */
-extern void setup_w32_kbdhook (void);
-extern void remove_w32_kbdhook (void);
+extern void setup_w32_kbdhook (HWND);
+extern void remove_w32_kbdhook (HWND);
 extern int check_w32_winkey_state (int);
 #define w32_kbdhook_active (os_subtype != OS_SUBTYPE_9X)
 #else