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

From: [email-masked] (GNU bug Tracking System)
Date: Thu, 14 Mar 2024 08:26:02 +0000

Your bug report

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

which was filed against the emacs package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to [email-masked].

-- 
69083: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=69083
GNU Bug Tracking System
Contact [email-masked] with problems

Subject: Re: bug#69083: Emacs's keyboard hook state is not reset on session lock (Windows)
From: Eli Zaretskii <[email-masked]>
To: Raffael Stocker <[email-masked]>
Cc: [email-masked]
Date: Thu, 14 Mar 2024 10:25:07 +0200
List-Id: <debbugs-submit.debbugs.gnu.org>

> From: Raffael Stocker <[email-masked]>
> Cc: [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.

Thanks.  I needed to fix this in small ways, to get it to compile with
mingw.org's MinGW, and I've now installed the results on the master
branch.

With that, I'm closing this bug.

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