[PATCH] server: Don't wait for low level hook result when queueing hardware message.
Without the change graphic drivers are blocking until low level hooks are processed when injecting keyboard and mouse events. That may cause deadlocks. Signed-off-by: Piotr Caban <piotr(a)codeweavers.com> --- server/queue.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
I'm attaching a simple application that may be used to observe the problem. The application creates a window and waits for WM_KEYDOWN message. It deadlocks without the patch. Unfortunately there's no way of testing it without the ability to generate non-injected hardware messages. I can add an interactive test if it's preferred. Thanks, Piotr
On 9/14/21 6:38 PM, Piotr Caban wrote:
I'm attaching a simple application that may be used to observe the problem. The application creates a window and waits for WM_KEYDOWN message. It deadlocks without the patch. Unfortunately there's no way of testing it without the ability to generate non-injected hardware messages. I can add an interactive test if it's preferred.
Thanks, Piotr
With an HID test signed driver, like I'm currently adding for dinput testing, we could test mouse and keyboard input without going through SendInput / kbd_event / mouse_event. -- Rémi Bernon <rbernon(a)codeweavers.com>
On 9/14/21 7:03 PM, Rémi Bernon wrote:
With an HID test signed driver, like I'm currently adding for dinput testing, we could test mouse and keyboard input without going through SendInput / kbd_event / mouse_event. The problem is that I need the event to originate from graphics driver to show the bug. It's currently the only path that calls ll hooks.
Thanks, Piotr
On 9/14/21 7:37 PM, Piotr Caban wrote:
On 9/14/21 7:03 PM, Rémi Bernon wrote:
With an HID test signed driver, like I'm currently adding for dinput testing, we could test mouse and keyboard input without going through SendInput / kbd_event / mouse_event. The problem is that I need the event to originate from graphics driver to show the bug. It's currently the only path that calls ll hooks.
Thanks, Piotr
Yes, of course. Then I think the issue may be more about the graphics driver architecture, where we poll and process X11 events in MsgWaitForMultipleObjects. I remember quite some time ago that for some reason I removed the wait for ll-hooks and julliard told me that it was needed for proper ll-hook processing. I'm still not much more knowledgeable about how ll-hooks work right now, but it may still be required, and the correct fix would be to have mouse and keyboard input coming from a separate source, as we do for other devices. It's a pretty big change and has a lot of implications though. -- Rémi Bernon <rbernon(a)codeweavers.com>
On 9/14/21 7:57 PM, Rémi Bernon wrote:
On 9/14/21 7:37 PM, Piotr Caban wrote:
On 9/14/21 7:03 PM, Rémi Bernon wrote:
With an HID test signed driver, like I'm currently adding for dinput testing, we could test mouse and keyboard input without going through SendInput / kbd_event / mouse_event. The problem is that I need the event to originate from graphics driver to show the bug. It's currently the only path that calls ll hooks.
Thanks, Piotr
Yes, of course.
Then I think the issue may be more about the graphics driver architecture, where we poll and process X11 events in MsgWaitForMultipleObjects.
I remember quite some time ago that for some reason I removed the wait for ll-hooks and julliard told me that it was needed for proper ll-hook processing.
I'm still not much more knowledgeable about how ll-hooks work right now, but it may still be required, and the correct fix would be to have mouse and keyboard input coming from a separate source, as we do for other devices.
It's a pretty big change and has a lot of implications though. I fail to see how it breaks ll-hooks processing. I've also done quite a lot of testing and it seems to be working good.
Yes, having __wine_send_input in separate thread/process will also fix the issue. If it's preferred to add something like wine keyboard driver that accepts key events from graphic drivers I can try to investigate it. I think that pumping X11 events in MsgWaitForMultipleObjects is a separate issue. No matter where we do the pumping we don't want it to hung on ll hook.
On 9/14/21 9:03 PM, Piotr Caban wrote:
On 9/14/21 7:57 PM, Rémi Bernon wrote:
On 9/14/21 7:37 PM, Piotr Caban wrote:
On 9/14/21 7:03 PM, Rémi Bernon wrote:
With an HID test signed driver, like I'm currently adding for dinput testing, we could test mouse and keyboard input without going through SendInput / kbd_event / mouse_event. The problem is that I need the event to originate from graphics driver to show the bug. It's currently the only path that calls ll hooks.
Thanks, Piotr
Yes, of course.
Then I think the issue may be more about the graphics driver architecture, where we poll and process X11 events in MsgWaitForMultipleObjects.
I remember quite some time ago that for some reason I removed the wait for ll-hooks and julliard told me that it was needed for proper ll-hook processing.
I'm still not much more knowledgeable about how ll-hooks work right now, but it may still be required, and the correct fix would be to have mouse and keyboard input coming from a separate source, as we do for other devices.
It's a pretty big change and has a lot of implications though. I fail to see how it breaks ll-hooks processing. I've also done quite a lot of testing and it seems to be working good.
I honestly don't know, and I haven't really investigated. I also thought everything was there on the server side to handle hooks properly but I may have missed something. Maybe things have evolved since then too, the remark was there: https://www.winehq.org/pipermail/wine-devel/2019-November/154392.html
Yes, having __wine_send_input in separate thread/process will also fix the issue. If it's preferred to add something like wine keyboard driver that accepts key events from graphic drivers I can try to investigate it.
Well feel free if you like, but I don't think it's a simple task. It has some implications to worry about, as if we have a lower-level source of keyboard / mouse input, we won't rely on X11 for event delivery and filtering, so we could start receiving unwanted input. It's something I had in mind for some long term exploration, and it may be possible to find simpler solution for this issue here.
I think that pumping X11 events in MsgWaitForMultipleObjects is a separate issue. No matter where we do the pumping we don't want it to hung on ll hook.
Well if the ll hooks aren't processed correctly, events are going to pile up somewhere. Either in X11 if the polling thread is blocked, or in wineserver if it isn't. I'm not sure it's better to have it in wineserver. -- Rémi Bernon <rbernon(a)codeweavers.com>
participants (3)
-
Piotr Caban -
Piotr Caban -
Rémi Bernon