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

[Raffael Stocker <r.stocker@mnet-mail.de>]

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