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

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

[Raffael Stocker]

[-- Attachment #1: Type: text/plain, Size: 2041 bytes --]


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



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: session notification patch --]
[-- Type: text/x-patch, Size: 3119 bytes --]

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

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

[Eli Zaretskii]

> From: Raffael Stocker <r.stocker@mnet-mail.de>
> Date: Mon, 12 Feb 2024 20:04:50 +0100
> 
> 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.

We can use that DLL, but we cannot link against it, because that would
make the DLL a prerequisite for running Emacs, and then it will be
impossible to start Emacs on Windows versions before XP, where that
DLL was introduced.  So we need to load that DLL at run time via
LoadLibrary, and only register for session notifications if the DLL is
available, by calling the DLL functions through function pointers --
this way, the Emacs executable doesn't have a static link-time
dependency on the DLL.  You will find many examples of this technique
in w32.c and in other w32*.c files in Emacs.

(I'm a bit surprised that Remote Desktop Services need to be used for
this purpose.  Are you sure there's no other way for Emacs to know
that the system is going to be locked?  Where did you read about the
need to reset the keyboard hook state in that case, and the way to do
it?)

> 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.

Please try the technique used by the function find_child_console which
is defined in w32proc.c.

> 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’.

Doesn't Windows remove the hook when the process exits?

> Also, in console mode the keyboard hook is always installed, while
> in GUI Emacs it is only installed when ‘WINDOWSNT’ is defined.

That's because w32console.c is not compiled into the Cygwin build
(which does not define WINDOWSNT), whereas w32fns.c is.

> @@ -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);

Do we really need to use WTSRegisterSessionNotificationEx?  Can't we
use WTSRegisterSessionNotification instead?  AFAIK, the latter is
available since Windows XP, whereas the former only since Vista.

Thanks.

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

[Raffael Stocker]

[-- Attachment #1: Type: text/plain, Size: 3958 bytes --]


Eli Zaretskii <eliz@gnu.org> writes:

> So we need to load that DLL at run time via LoadLibrary, ...

I have added that.

> (I'm a bit surprised that Remote Desktop Services need to be used for
> this purpose.  Are you sure there's no other way for Emacs to know
> that the system is going to be locked?  Where did you read about the
> need to reset the keyboard hook state in that case, and the way to do
> it?)

For me, the need to reset the hook state is implied by the existence of
the ‘reset_w32_kbdhook_state’ function defined in ‘w32fns.c’ and its
call site.  This function has been there since 2016, but was never
called, because the ‘WM_WTSSESSION_CHANGE’ message wasn't received.
From [0]:

MS> This message is sent only to applications that have registered to
MS> receive this message by calling WTSRegisterSessionNotification.

This seems to be the only way to find out it's being locked; at least I
couldn't find anything indicating otherwise on the Microsoft website and
a quick web search didn't turn up anything either.

>> 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.
>
> Please try the technique used by the function find_child_console which
> is defined in w32proc.c.

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.  I must be doing
something wrong here.  OTOH, [1] says:

MS> Note For Windows 8 and later, EnumWindows enumerates only top-level
MS> windows of desktop apps.

Does this mean that ‘EnumWindows’ it doesn't work for console windows or
is this remark irrelevant here?  I am not familiar with the Windows
terminology.

Come to think of it, don't I need the window handle of the console
window that Emacs is running in, which would imply that a handle for
Emacs' pid doesn't exist?  I was starting Emacs in cmd.exe using ‘emacs
-Q -nw’, so I would probably have to find the pid of the ‘cmd’ process
first, right?

>> 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’.
>
> Doesn't Windows remove the hook when the process exits?

According to [2] we have to remove the hook:

MS> Before terminating, an application must call the UnhookWindowsHookEx
MS> function function to free system resources associated with the hook.

>> Also, in console mode the keyboard hook is always installed, while
>> in GUI Emacs it is only installed when ‘WINDOWSNT’ is defined.
>
> That's because w32console.c is not compiled into the Cygwin build
> (which does not define WINDOWSNT), whereas w32fns.c is.

Ok, that makes sense, I have removed my superfluous #ifdef.

> Do we really need to use WTSRegisterSessionNotificationEx?  Can't we
> use WTSRegisterSessionNotification instead?  AFAIK, the latter is
> available since Windows XP, whereas the former only since Vista.

Yes, ‘WTSRegisterSessionNotification’ works, too.  However, [3] says
that the minimum supported client here is Windows Vista, as with the
‘-Ex’ version.

I attached my current patch version.

Regards,
Raffael

[0] https://learn.microsoft.com/en-us/windows/win32/termserv/wm-wtssession-change
[1] https://learn.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-enumwindows
[2] https://learn.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-setwindowshookexa
[3] https://learn.microsoft.com/en-us/windows/win32/api/wtsapi32/nf-wtsapi32-wtsregistersessionnotification


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: reset kbdhook patch --]
[-- Type: text/x-patch, Size: 3716 bytes --]

diff --git a/src/w32console.c b/src/w32console.c
index 0936b5f37e6..2161e738252 100644
--- a/src/w32console.c
+++ b/src/w32console.c
@@ -822,7 +822,9 @@ initialize_w32_display (struct terminal *term, int *width, int *height)
   w32_initialize_display_info (build_string ("Console"));
 
   /* 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);
 }
 
 
diff --git a/src/w32fns.c b/src/w32fns.c
index 8d4bd00b91c..1f2a68c2fed 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
@@ -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;
@@ -2744,7 +2749,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 +2805,35 @@ setup_w32_kbdhook (void)
       /* Set the hook.  */
       kbdhook.hook = SetWindowsHookEx (WH_KEYBOARD_LL, funhook,
 				       GetModuleHandle (NULL), 0);
+      if (hwnd != NULL)
+	{
+	  HMODULE wtsapi32_lib = LoadLibrary ("wtsapi32.dll");
+	  WTSRegisterSessionNotification_Proc WTSRegisterSessionNotification_fn
+	    = (WTSRegisterSessionNotification_Proc)
+	    get_proc_addr (wtsapi32_lib, "WTSRegisterSessionNotification");
+	  if (WTSRegisterSessionNotification_fn != NULL)
+	    WTSRegisterSessionNotification_fn (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)
+	{
+	  HMODULE wtsapi32_lib = GetModuleHandle ("wtsapi32.dll");
+	  WTSUnRegisterSessionNotification_Proc WTSUnRegisterSessionNotification_fn
+	    = (WTSUnRegisterSessionNotification_Proc)
+	    get_proc_addr (wtsapi32_lib, "WTSUnRegisterSessionNotification");
+	  if (WTSUnRegisterSessionNotification_fn != NULL)
+	    WTSUnRegisterSessionNotification_fn (hwnd);
+	}
       kbdhook.hook = NULL;
     }
 }
@@ -5301,13 +5324,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

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

[Eli Zaretskii]

> From: Raffael Stocker <r.stocker@mnet-mail.de>
> Cc: 69083@debbugs.gnu.org
> Date: Wed, 14 Feb 2024 21:20:35 +0100
> 
> > So we need to load that DLL at run time via LoadLibrary, ...
> 
> I have added that.

Thanks.  I suggest one optimization: instead of probing whether
LoadLibrary succeeds in two places, do it only once, where you call
WTSRegisterSessionNotification, and at that time also record the
function pointer for WTSUnRegisterSessionNotification; then use the
2nd function pointer, if non-NULL, to call the unregister API.

> > (I'm a bit surprised that Remote Desktop Services need to be used for
> > this purpose.  Are you sure there's no other way for Emacs to know
> > that the system is going to be locked?  Where did you read about the
> > need to reset the keyboard hook state in that case, and the way to do
> > it?)
> 
> For me, the need to reset the hook state is implied by the existence of
> the ‘reset_w32_kbdhook_state’ function defined in ‘w32fns.c’ and its
> call site.  This function has been there since 2016, but was never
> called, because the ‘WM_WTSSESSION_CHANGE’ message wasn't received.
> >From [0]:
> 
> MS> This message is sent only to applications that have registered to
> MS> receive this message by calling WTSRegisterSessionNotification.
> 
> This seems to be the only way to find out it's being locked; at least I
> couldn't find anything indicating otherwise on the Microsoft website and
> a quick web search didn't turn up anything either.

This URL:

  https://stackoverflow.com/questions/8606300/how-to-detect-windows-is-locked

seems to indicate that WTSRegisterSessionNotification requires
elevation on Windows 10/11.  Did you get the UAC prompt when calling
that API?  Are you running with admin privileges when you test this
code?  Triggering UAC prompts when starting Emacs would be a nuisance
to our users.

> >> 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.
> >
> > Please try the technique used by the function find_child_console which
> > is defined in w32proc.c.
> 
> 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 must be doing something wrong here.  OTOH, [1] says:
> 
> MS> Note For Windows 8 and later, EnumWindows enumerates only top-level
> MS> windows of desktop apps.
> 
> Does this mean that ‘EnumWindows’ it doesn't work for console windows or
> is this remark irrelevant here?  I am not familiar with the Windows
> terminology.

I think it does work with console windows, since that's what Emacs
uses it for (the sub-processes Emacs runs are almost always console
processes).

> Come to think of it, don't I need the window handle of the console
> window that Emacs is running in, which would imply that a handle for
> Emacs' pid doesn't exist?  I was starting Emacs in cmd.exe using ‘emacs
> -Q -nw’, so I would probably have to find the pid of the ‘cmd’ process
> first, right?

Maybe, I don't know.  But first, I'd like to understand what goes
wrong when you are looking for a window for which
GetWindowThreadProcessId returns the PID of Emacs.

> > Do we really need to use WTSRegisterSessionNotificationEx?  Can't we
> > use WTSRegisterSessionNotification instead?  AFAIK, the latter is
> > available since Windows XP, whereas the former only since Vista.
> 
> Yes, ‘WTSRegisterSessionNotification’ works, too.  However, [3] says
> that the minimum supported client here is Windows Vista, as with the
> ‘-Ex’ version.

That's a lie.  The MS docs frequently pretend that old versions of
Windows don't exist, and this is one such case.

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

[Raffael Stocker]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Raffael Stocker <r.stocker@mnet-mail.de>
>> Cc: 69083@debbugs.gnu.org
>> Date: Wed, 14 Feb 2024 21:20:35 +0100
>> 
>> > So we need to load that DLL at run time via LoadLibrary, ...
>> 
>> I have added that.
>
> Thanks.  I suggest one optimization: instead of probing whether
> LoadLibrary succeeds in two places, do it only once, where you call
> WTSRegisterSessionNotification, and at that time also record the
> function pointer for WTSUnRegisterSessionNotification; then use the
> 2nd function pointer, if non-NULL, to call the unregister API.

Yes, I can do that.  I will leave the initialisation in the
setup_w32_kbdhook function as the branch with the call to LoadLibrary
should be executed only once.

I experimented a bit today and found that the remove_w32_kbdhook
function is not actually getting called when Emacs is being killed.  It
is only called when the Emacs window receives a WM_DESTROY message.  But
we get relevant WM_* messages, when

   - creating second frame with ‘C-x 5 2’ and closing it using ‘C-x 5 0’:
     WM_EMACS_DESTROYWINDOW, then WM_DESTROY
   - creating second frame with ‘C-x 5 2’ and closing it by clicking ‘X’
     in frame decoration:
     WM_CLOSE, then WM_EMACS_DESTROYWINDOW, then WM_DESTROY
   - closing the last frame by clicking ‘X’ in frame decoration: WM_CLOSE
   - killing Emacs with ‘C-x C-c’: nothing

At least, if Emacs is not run as daemon.  Then, the WM_DESTROY is only
handled when there is still another frame, and in this case
‘remove_w32_kbdhook’ will not remove the hook as kbdhook.hook_count > 1.

If Emacs is run as daemon, it cleans up the keyboard hook when closing
the last window.

Is there a better place where the remove_w32_kbdhook call could go such
that cleanup can always happen?  Or should we just not care?

> This URL:
>
>   https://stackoverflow.com/questions/8606300/how-to-detect-windows-is-locked
>
> seems to indicate that WTSRegisterSessionNotification requires
> elevation on Windows 10/11.  Did you get the UAC prompt when calling
> that API?  Are you running with admin privileges when you test this
> code?  Triggering UAC prompts when starting Emacs would be a nuisance
> to our users.

I have read this post, but it seems to be wrong.  I don't have any
elevated privileges and it works without showing any prompt, at least on
Windows 10.  I'll try to have my colleague test it as well, she runs
Windows 11.

>> 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'm not quite sure myself what I mean.  I will experiment with this
a bit more and try to find out what's happening.

Regards,
Raffael

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

[Eli Zaretskii]

> From: Raffael Stocker <r.stocker@mnet-mail.de>
> Cc: 69083@debbugs.gnu.org
> Date: Tue, 20 Feb 2024 19:51:21 +0100
> 
> I experimented a bit today and found that the remove_w32_kbdhook
> function is not actually getting called when Emacs is being killed.  It
> is only called when the Emacs window receives a WM_DESTROY message.  But
> we get relevant WM_* messages, when
> 
>    - creating second frame with ‘C-x 5 2’ and closing it using ‘C-x 5 0’:
>      WM_EMACS_DESTROYWINDOW, then WM_DESTROY
>    - creating second frame with ‘C-x 5 2’ and closing it by clicking ‘X’
>      in frame decoration:
>      WM_CLOSE, then WM_EMACS_DESTROYWINDOW, then WM_DESTROY
>    - closing the last frame by clicking ‘X’ in frame decoration: WM_CLOSE
>    - killing Emacs with ‘C-x C-c’: nothing
> 
> At least, if Emacs is not run as daemon.  Then, the WM_DESTROY is only
> handled when there is still another frame, and in this case
> ‘remove_w32_kbdhook’ will not remove the hook as kbdhook.hook_count > 1.
> 
> If Emacs is run as daemon, it cleans up the keyboard hook when closing
> the last window.
> 
> 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.

> > This URL:
> >
> >   https://stackoverflow.com/questions/8606300/how-to-detect-windows-is-locked
> >
> > seems to indicate that WTSRegisterSessionNotification requires
> > elevation on Windows 10/11.  Did you get the UAC prompt when calling
> > that API?  Are you running with admin privileges when you test this
> > code?  Triggering UAC prompts when starting Emacs would be a nuisance
> > to our users.
> 
> I have read this post, but it seems to be wrong.  I don't have any
> elevated privileges and it works without showing any prompt, at least on
> Windows 10.  I'll try to have my colleague test it as well, she runs
> Windows 11.

Is your user a member of the local Administrators group?  Did you try
running this code as a "normal" user, not an admin?

> >> 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'm not quite sure myself what I mean.  I will experiment with this
> a bit more and try to find out what's happening.

Thanks.

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

[Raffael Stocker]

Eli Zaretskii <eliz@gnu.org> writes:
>> I have read this post, but it seems to be wrong.  I don't have any
>> elevated privileges and it works without showing any prompt, at least on
>> Windows 10.  I'll try to have my colleague test it as well, she runs
>> Windows 11.
>
> Is your user a member of the local Administrators group?  Did you try
> running this code as a "normal" user, not an admin?

I did run it as a normal user; my user has no (local or otherwise) admin
rights.

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

[Eli Zaretskii]

> From: Raffael Stocker <r.stocker@mnet-mail.de>
> Cc: 69083@debbugs.gnu.org
> Date: Wed, 21 Feb 2024 15:13:30 +0100
> 
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> >> I have read this post, but it seems to be wrong.  I don't have any
> >> elevated privileges and it works without showing any prompt, at least on
> >> Windows 10.  I'll try to have my colleague test it as well, she runs
> >> Windows 11.
> >
> > Is your user a member of the local Administrators group?  Did you try
> > running this code as a "normal" user, not an admin?
> 
> I did run it as a normal user; my user has no (local or otherwise) admin
> rights.

Then it's possible that members of Administrators will get a UAC.  We
shall see, I guess.

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

[Raffael Stocker]

[-- Attachment #1: Type: text/plain, Size: 1956 bytes --]


Eli Zaretskii <eliz@gnu.org> 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


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: reset_kbdhook_state patch --]
[-- Type: text/x-patch, Size: 7028 bytes --]

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);
     }

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

[Eli Zaretskii]

> From: Raffael Stocker <r.stocker@mnet-mail.de>
> Cc: 69083@debbugs.gnu.org
> Date: Mon, 26 Feb 2024 21:50:43 +0100
> 
> Eli Zaretskii <eliz@gnu.org> 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.

Thanks, I have a few minor comments to the patch below.  After fixing
those nits, would you please submit the result in "git format-patch"
format, and include commit log message according to our conventions
(see CONTRIBUTE for the details and "git log" to see examples of how
we do that in the repository).  This will allow me to install the
changes quickly and easily.

> +/* from w32fns.c */
> +extern void
> +remove_w32_kbdhook (void);

Please make the prototype a single line, don't break it in two as we
do with function definitions.

> +/* The IME window is needed to receive the session notifications
> +   required to reset the low level keyboard hook state. */
                                                         ^^
Our style is to leave two SPACEs at the end of a comment.

> +      /* 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);
> +	}

This code is run every time Emacs creates a new frame, doesn't it?  If
so, calling LoadLibrary and get_proc_addr each time is a waste of
cycles.  It is better to make both WTSRegisterSessionNotification_fn
and WTSUnRegisterSessionNotification_fn global, and introduce a
boolean flag to indicate that this was already done.  Then the above
code should be run only once per session, and all the other calls
should use the function pointers already set up (if non-NULL).

Note that the boolean flag and the function pointers need to be reset
at startup in globals_of_w32, because we still support the unexec
build of Emacs, where global and static variables set during the
temacs run as part of the build are recorded in the emacs.exe binary,
and will therefore record the values from the build time, which are
not necessarily true for the run time of production sessions (which
could well be on another system).

OTOH, there's something I don't understand here.  If this code is run
for every frame we create/delete, then what window exactly does the
kbdhook.window member record?  It sounds like we overwrite that member
with another window's handle on each call to setup_w32_kbdhook?  And
if so, what is the window handle we will pass to
WTSUnRegisterSessionNotification in remove_w32_kbdhook?  Or is the
hwnd argument to setup_w32_kbdhook somehow non-NULL only once, for the
main session window or something?  I feel that I'm missing something
here.

Thanks.

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

[Raffael Stocker]

Eli Zaretskii <eliz@gnu.org> writes:
>> +      /* 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);
>> +	}
>
> This code is run every time Emacs creates a new frame, doesn't it?  If
> so, calling LoadLibrary and get_proc_addr each time is a waste of
> cycles.  It is better to make both WTSRegisterSessionNotification_fn
> and WTSUnRegisterSessionNotification_fn global, and introduce a
> boolean flag to indicate that this was already done.  Then the above
> code should be run only once per session, and all the other calls
> should use the function pointers already set up (if non-NULL).

The code is run when a first frame is created, because only then
‘kbdhook.hook_count’ equals 1.  If Emacs runs as daemon, this can happen
several times in a session, when the user deletes all frames and then
creates a new one again.

> OTOH, there's something I don't understand here.  If this code is run
> for every frame we create/delete, then what window exactly does the
> kbdhook.window member record?  It sounds like we overwrite that member
> with another window's handle on each call to setup_w32_kbdhook?  And
> if so, what is the window handle we will pass to
> WTSUnRegisterSessionNotification in remove_w32_kbdhook?  Or is the
> hwnd argument to setup_w32_kbdhook somehow non-NULL only once, for the
> main session window or something?  I feel that I'm missing something
> here.

You are right, this is iffy.  Setting up the hook for the first frame
works well, as the hook is per-process.  I thought I could use the same
approach here, but the session notifications are per-window.  So if the
user creates two frames, the session notification is registered for the
first one only.  If she deletes the first frame (leaving the second one
alone), the notification gets unregistered and the hook reset is not
called anymore upon session lock.

So, I guess the options I have are either somehow juggling window
handles, making sure the session notification is always registered for
exactly one window handle no matter how many frames are created and
deleted (so ‘reset_w32_kbdhook_state’ is called exactly once for every
session lock), or registering the notification for every frame.  In the
latter case I would either have to find a way to ensure the reset
function is only called once, or just not care that it is called for
every existing frame.  The latter option is simple, but a bit dirty.
OTOH, the reset function only sets the state variables to zero, so there
shouldn't be a problem with this approach.

I'll think about this a bit more, maybe I can come up with a nice
solution that doesn't require keeping too much state just for the reset
function.  I am thinking about registering the notification for the
first created frame, and when that is deleted, i.e. receives the
‘WM_DESTROY’ message, "handing" it over to some other frame if there is
one.  I don't know much about the internals of Emacs frame handling.
Could ‘w32_frame_list_z_order’ be used (from the input thread) for
something like that?  Or is there a better approach?

Regards,
Raffael

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

[Eli Zaretskii]

> From: Raffael Stocker <r.stocker@mnet-mail.de>
> Cc: 69083@debbugs.gnu.org
> Date: Thu, 29 Feb 2024 21:22:13 +0100
> 
> I'll think about this a bit more, maybe I can come up with a nice
> solution that doesn't require keeping too much state just for the reset
> function.  I am thinking about registering the notification for the
> first created frame, and when that is deleted, i.e. receives the
> ‘WM_DESTROY’ message, "handing" it over to some other frame if there is
> one.

This sounds like the simplest approach to me.

> I don't know much about the internals of Emacs frame handling.
> Could ‘w32_frame_list_z_order’ be used (from the input thread) for
> something like that?  Or is there a better approach?

Why is the order important?  Can't you "hand" the registration to some
other frame, no matter which one?

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

[Raffael Stocker]

[-- Attachment #1: Type: text/plain, Size: 1111 bytes --]

Eli Zaretskii <eliz@gnu.org> writes:

>> I'll think about this a bit more, maybe I can come up with a nice
>> solution that doesn't require keeping too much state just for the reset
>> function.  I am thinking about registering the notification for the
>> first created frame, and when that is deleted, i.e. receives the
>> ‘WM_DESTROY’ message, "handing" it over to some other frame if there is
>> one.
>
> This sounds like the simplest approach to me.

I have implemented this and attached the patch.  I have tested it with
both console and GUI Emacs (with and without server) and have found no
further problems with it.

>> I don't know much about the internals of Emacs frame handling.
>> Could ‘w32_frame_list_z_order’ be used (from the input thread) for
>> something like that?  Or is there a better approach?
>
> Why is the order important?  Can't you "hand" the registration to some
> other frame, no matter which one?

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’.

Regards,
Raffael


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Fix resetting keyboard hook state --]
[-- Type: text/x-patch, Size: 10101 bytes --]

From fb9b8daf18a3dc96879d58d9c4e1e3dcfcf8e2e7 Mon Sep 17 00:00:00 2001
From: Raffael Stocker <r.stocker@mnet-mail.de>
Date: Sun, 3 Mar 2024 17:31:10 +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     | 73 +++++++++++++++++++++++++++++++++++++++++++++---
 src/w32term.h    |  3 +-
 src/w32xfns.c    | 12 ++++++--
 5 files changed, 109 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..32cc466d9eb 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,36 @@ #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 frame = Fnext_frame (Qnil, Qnil);
+      struct frame *f = XFRAME (frame);
+
+      if (FRAME_OUTPUT_DATA (f) != NULL
+	  && 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 +5352,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 +11392,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

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

[Eli Zaretskii]

> From: Raffael Stocker <r.stocker@mnet-mail.de>
> Cc: 69083@debbugs.gnu.org
> Date: Sun, 03 Mar 2024 17:43:56 +0100
> 
> >> I don't know much about the internals of Emacs frame handling.
> >> Could ‘w32_frame_list_z_order’ be used (from the input thread) for
> >> something like that?  Or is there a better approach?
> >
> > Why is the order important?  Can't you "hand" the registration to some
> > other frame, no matter which one?
> 
> 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?

Otherwise, I think the patch is fine, at least from reading it.

Thanks.

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

[Eli Zaretskii]

> Cc: 69083@debbugs.gnu.org
> Date: Sun, 03 Mar 2024 19:23:05 +0200
> From: Eli Zaretskii <eliz@gnu.org>
> 
> Unfortunately, I don't think you cannot use Fnext_frame here.  The
                                   ^^^^^^
That was supposed to be "can", not "cannot"...

Sorry.

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

[Raffael Stocker]

[-- Attachment #1: Type: text/plain, Size: 1223 bytes --]

Eli Zaretskii <eliz@gnu.org> 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


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Fix keyboard hook state reset --]
[-- Type: text/x-patch, Size: 10296 bytes --]

From 93e6446d3e75b990230618f9d3331fd5a0818d50 Mon Sep 17 00:00:00 2001
From: Raffael Stocker <r.stocker@mnet-mail.de>
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

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

[Eli Zaretskii]

> From: Raffael Stocker <r.stocker@mnet-mail.de>
> Cc: 69083@debbugs.gnu.org
> Date: Mon, 04 Mar 2024 19:10:33 +0100
> 
> Eli Zaretskii <eliz@gnu.org> 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.