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

From: Raffael Stocker <[email-masked]>
Date: Thu, 29 Feb 2024 21:22:13 +0100

Eli Zaretskii <[email-masked]> 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