Problem with bad generation of low-level keyboard events

[Lionel Ulmer]

Hi all,

While debugging GeneRally (a very nice game), I got some problems with
DInput receiving spurious events (basically, as if key '0' was pressed).

This is traced to the following problem :

In windows/input.c, the function 'queue_kbd_event' stores the
KBDLLHOOKSTRUCT structure on the stack and then calls 'HOOK_CallHooks' to
send it. In my case, thread 000a receives the event and sends it to thread
0009.

What happens is the following (the 'Hook:' lines were added by me) :

000a:trace:hook:HOOK_CallHooks calling hook in thread 0009 WH_KEYBOARD_LL code 0 wp 101 lp 4156ad6c
000a:trace:hook:HOOK_CallHooks Hook: 0x4156ad6c - 00000026 00000048 000000c1 0000b578 00000000
000a:trace:msg:send_inter_thread_message hwnd (nil) msg 80000007 ("#7") wp 101 lp 4156ad6c
000a: send_message( id=0009, type=1, flags=1, win=(nil), msg=80000007, wparam=00000101, lparam=4156ad6c, x=0, y=0, time=0000b587, info=00000000, timeout=2000, callback=(nil), data={} )
000a: send_message() = 0
000a: set_queue_mask( wake_mask=00008040, changed_mask=00008040, skip_wait=1 )
000a: set_queue_mask() = 0 { wake_bits=00000000, changed_bits=00000000 }
000a: select( flags=4, cookie=0x4156a578, timeout=0, handles={0x50} )
000a: select() = PENDING
0009: get_handle_fd( handle=0xc0, access=80000000 )
0009: get_handle_fd() = 0 { fd=33, type=1, flags=0 }
(...)
0009:trace:ddraw:Main_DirectDrawSurface_Lock locked surface returning
description : 
 - DDSD_CAPS : DDSCAPS_OFFSCREENPLAIN DDSCAPS_VIDEOMEMORY 
(...)
000a: *wakeup* signaled=0 cookie=0x4156a578
000a: set_queue_mask( wake_mask=00008040, changed_mask=00008040, skip_wait=1 )
000a: set_queue_mask() = 0 { wake_bits=00008000, changed_bits=00008000 }
000a: get_message_reply( cancel=1 )
000a: get_message_reply() = TIMEOUT { result=00000000, data={} }
000a:trace:msg:retrieve_reply hwnd (nil) msg 80000007 ("#7") wp 101 lp 4156ad6c got reply 40218182 (err=258)
000a: finish_hook_chain( id=13 )
000a: finish_hook_chain() = 0
000a:trace:key:queue_kbd_event Hook: 1) 0x4156ad6c - 00000026 00000048 000000c1 0000b578 00000000
000a: send_message( id=000a, type=7, flags=0, win=(nil), msg=00000101, wparam=00000026, lparam=c1480001, x=691, y=584, time=0000b578, info=00000000, timeout=-1, callback=(nil), data={} )
000a: send_message() = 0
000a:trace:key:queue_kbd_event Hook: 2) 0x4156ad6c - 00000026 00000048 000000c1 0000b578 00000000
000a:trace:event:EVENT_ProcessEvent returns.
(...)
0009: select( flags=12, cookie=0x4069f890, timeout=1081894976.490610 (-0.000077), handles={0x3c} )
0009: select() = TIMEOUT
0009: get_message( flags=1, get_win=(nil), get_first=00000000, get_last=ffffffff )
0009: get_message() = 0 { type=1, win=(nil), msg=80000007, wparam=00000101, lparam=4156ad6c, x=0, y=0, time=0000b587, info=00000000, total=0, data={} }
0009:trace:msg:MSG_peek_message got type 1 msg 80000007 ("#7") hwnd (nil) wp 101 lp 4156ad6c
0009: start_hook_chain( id=13 )
0009: start_hook_chain() = 0 { handle=0x10022, pid=0000, tid=0000, proc=0x40cd4c70, unicode=1, module=L"" }
0009:trace:hook:HOOK_CallHooks calling hook 0x40cd4c70 WH_KEYBOARD_LL code 0 wp 101 lp 4156ad6c module L""
0009:trace:hook:HOOK_CallHooks Hook: 0x4156ad6c - 4156ad88 00000000 00000040 4156ad8c 4156ad88

A summary of my understanding of the problem :

 - in 'queue_kbd_event', the structure is put on the stack on the
   expectation that the 'HOOK_CallHooks' won't return with the message
   unsent.

 - as seen, because the thread which receives the message does not seem to
   handle any messages during quite a long time (it's actually loading the
   track between two races), this message sending time-outs on the sender's
   side.

 - so the function 'queue_kbd_event' ends and the stack gets corrupted.
 
 - which can be seen in my last 'Hook:' dump.

If it's normal for the sending to time-out, we should replace this code and
add some HeapAlloc on the sender side and some HeapFree on the receiver side
each time such a structure is sent ... Which will be a bit problematic for
performance, but well, I do not see any other way to correct this.

         Lionel

-- 
		 Lionel Ulmer - http://www.bbrox.org/

[Alexandre Julliard]

Lionel Ulmer <lionel.ulmer(a)free.fr> writes:
> If it's normal for the sending to time-out, we should replace this code and
> add some HeapAlloc on the sender side and some HeapFree on the receiver side
> each time such a structure is sent ... Which will be a bit problematic for
> performance, but well, I do not see any other way to correct this.

The real bug is that the thread still receives the message even though
it has timed out on the sender side. The message should be cancelled
in that case. I'll have a look at fixing that.

-- 
Alexandre Julliard
julliard(a)winehq.org

[Lionel Ulmer]

> The real bug is that the thread still receives the message even though
> it has timed out on the sender side. The message should be cancelled
> in that case. I'll have a look at fixing that.

This will fix the 'bad key-press' event bug... But it will still mean that
the game will miss a keyboard event (and in this case, it's the 'KeyRelease'
event which will trigger exactly the same bug as before).

      Lionel

-- 
		 Lionel Ulmer - http://www.bbrox.org/