From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Eli Zaretskii Newsgroups: gmane.emacs.bugs Subject: bug#69083: Emacs's keyboard hook state is not reset on session lock (Windows) Date: Tue, 27 Feb 2024 09:42:34 +0200 Message-ID: <868r36uzdh.fsf@gnu.org> References: <86cyt1qvmt.fsf@gnu.org> <861q9exmhe.fsf@gnu.org> <86v86im1r9.fsf@gnu.org> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="27707"; mail-complaints-to="usenet@ciao.gmane.io" Cc: 69083@debbugs.gnu.org To: Raffael Stocker Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Tue Feb 27 08:45:48 2024 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1res9r-0006t8-S2 for geb-bug-gnu-emacs@m.gmane-mx.org; Tue, 27 Feb 2024 08:45:48 +0100 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1res9j-0003x9-27; Tue, 27 Feb 2024 02:45:39 -0500 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1res9h-0003wq-63 for bug-gnu-emacs@gnu.org; Tue, 27 Feb 2024 02:45:37 -0500 Original-Received: from debbugs.gnu.org ([2001:470:142:5::43]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1res9g-0003Dh-IB for bug-gnu-emacs@gnu.org; Tue, 27 Feb 2024 02:45:36 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1resA6-0006HM-BF for bug-gnu-emacs@gnu.org; Tue, 27 Feb 2024 02:46:02 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Eli Zaretskii Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Tue, 27 Feb 2024 07:46:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 69083 X-GNU-PR-Package: emacs Original-Received: via spool by 69083-submit@debbugs.gnu.org id=B69083.170901992324012 (code B ref 69083); Tue, 27 Feb 2024 07:46:02 +0000 Original-Received: (at 69083) by debbugs.gnu.org; 27 Feb 2024 07:45:23 +0000 Original-Received: from localhost ([127.0.0.1]:58636 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1res9S-0006FD-LT for submit@debbugs.gnu.org; Tue, 27 Feb 2024 02:45:23 -0500 Original-Received: from eggs.gnu.org ([209.51.188.92]:52594) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1res9Q-0006En-3o for 69083@debbugs.gnu.org; Tue, 27 Feb 2024 02:45:21 -0500 Original-Received: from fencepost.gnu.org ([2001:470:142:3::e]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1res6o-0002Ub-RI; Tue, 27 Feb 2024 02:42:38 -0500 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnu.org; s=fencepost-gnu-org; h=MIME-version:References:Subject:In-Reply-To:To:From: Date; bh=eaIUv71jKzysEDqoPKxp4jvJG7KUhtjpB/eK6SKD23E=; b=Xab71CPUNjF4vQs6sAWf x88nC/4PUjz0+FapdTfg+S7cY9HongerBVL4812W1JISzjNPLEIlBuAIqMJ5qvJn4RJkwOgfh8A1o lJIlqSSqEbaQXvV4Mgl3/aD1g6TrMQmP9Vw5N6GZN2U31FMvKNscjPy5bbfLI3hCxBOHfUusM2I9J e7KZJ+N2TwndsLwy8fob61kMo7YXJ0V35NSBL7IdVm7xVIMnKQCJvnkfy0GMvrK9j19+4XjRuuoy8 b71QyDSLKDe+mxkZegZbB3Sp8QEQbFbW+oauJrucaMLbfDasHaAfCRFBThLIMlVxxsa+u5Fec1LHP tE8lpm2eOxWMVw==; In-Reply-To: (message from Raffael Stocker on Mon, 26 Feb 2024 21:50:43 +0100) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.bugs:280715 Archived-At: > From: Raffael Stocker > Cc: 69083@debbugs.gnu.org > Date: Mon, 26 Feb 2024 21:50:43 +0100 > > Eli Zaretskii 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.