From 56e727e0265c4480f6dcc4a3649c62277fcd0d2a Mon Sep 17 00:00:00 2001 From: Uli Schlachter Date: Thu, 16 Aug 2018 18:03:22 +0200 Subject: [PATCH] Fix a race with setting the focus There are two ways in which the input focus can change: Lua can request a change and the X11 server can inform us that the input focus changed (because some application changed it). In the first case, we still have to inform the X11 server about the desired change, in the second case we must not to avoid races due to X11's asynchronous nature. However, there was a case where we screwed up: When a focus change is still pending, meaning that Lua assigned the focus elsewhere, but we have not yet sent this focus change to the X11 server, we could get an event from the X11 server telling us that the focus changed. To make sure that the pending focus change is not lost, we sent the focus change out in this case (call to client_focus_refresh() in event_handle_focusin()). After sending out this pending call, we updated the internal state to record that whatever the X11 server just told us had the focus. The intention was that our just sent-out focus change will cause the X11 server to send a new event and our to-be-focused client then has the focus. However, if the pending focus change was for a client which only showed up in this event loop iteration, the client was still banned. This means that client_focus_refresh() would call client_unban() to be able to give the focus to this client. However, since awesome (partly) allows to "focus" currently banned clients, client_unban() recorded that there is a pending focus change. This caused confusion later on. In this specific bug, a main window opened a dialog, and when this dialog was closed, a new dialog window was opened immediately. When the first dialog was closed, Lua (the focus history) gave the input focus to the main window. Now, a new dialog showed up and Lua focused it. Next, we received the event from the X11 server telling us that the main window was focused. Because there was still a pending focus change to the new dialog window, event_handle_focusin() called client_focus_refresh() to send out this focus change. This set globalconf.focus.need_update to false and continued. However, because the new dialog only just now appeared, it was still banned, meaning that client_focus_refresh() had to call client_unban(). This set globalconf.focus.need_update to true. Thus, when client_focus_refresh() returned, globalconf.focus.need_update was incorrectly true. Next, event_handle_focusin() recorded that the main window had the focus. Thus, it now appeared as if there was a pending focus change for the main window. Next, we got the event from the X11 server telling us that the dialog is now focused, and because focus.need_update was set, awesome now send out a focus change request for the main window. Fix this race by unsetting globalconf.focus.need_update at the end of client_focus_refresh() and not at the beginning, thus making sure that client_unban() cannot set this flag again. Fixes: https://github.com/awesomeWM/awesome/issues/2220 Signed-off-by: Uli Schlachter --- objects/client.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/objects/client.c b/objects/client.c index 817205638..b893da608 100644 --- a/objects/client.c +++ b/objects/client.c @@ -1242,7 +1242,6 @@ client_focus_refresh(void) if(!globalconf.focus.need_update) return; - globalconf.focus.need_update = false; if(c && client_on_selected_tags(c)) { @@ -1265,6 +1264,9 @@ client_focus_refresh(void) */ xcb_set_input_focus(globalconf.connection, XCB_INPUT_FOCUS_PARENT, win, globalconf.timestamp); + + /* Do this last, because client_unban() might set it to true */ + globalconf.focus.need_update = false; } static void