Re: bug report/suggested temp. patch: handling bursts of sent keys

[Jon TURNEY <jon dot turney at dronecode dot org dot uk>]

On 12/01/2010 18:44, Mark Lillibridge wrote:
> Background:
>
>      I use Nuance's Dragon NaturallySpeaking voice recognition software
> to control remote Linux systems from a Windows box.  I open up xterms
> and emacs windows on a local Cygwin X server.  Occasionally the voice
> recognizer makes a mistake, which has to be corrected.  The mechanics of
> how this is done are unimportant; what matters is how Dragon applies the
> correction.  It does this by sending a large burst of keystrokes via
> something like SendKeys.  (E.g., many (shift) arrow keys to select the
> text to be changed, a backspace to delete it, then the replacement
> text.)
>
>      Unfortunately, there is a bug in the current released Cygwin X
> server that causes it to drop sent keystrokes when the burst exceeds a
> given size (roughly 64 or 128 keys depending on the version).  This
> breaks correction, and forces painful manual fix up of the text.  Note
> that what is really dropped are key events so the system can get into
> the state where the shift key remains pressed or a key starts repeating
> forever because the key up event was dropped.
[repro snipped]

Mark,

Thanks for the detailed bug report and clear reproduction steps.

On 23/01/2010 22:02, Mark Lillibridge wrote:
>      I am not a Windows programmer.  Can someone tell me if it's okay for
> winWindowProc to block?  In particular, could we make it block until the
> mieq queue is not full?

I think blocking would just result in a deadlock, as the X server is only 
single-threaded.  The windows message pump is called when the server has no 
other work to do.

This should be documented in [1], although perhaps that is lacking in detail.

I notice that winWakeHandler()/winBlockHandler() try to completely empty the 
windows message queue, which leads to this problem as the server won't get a 
chance to process anything (draining the mieq queue) until they return.

It might be enough to resolve this problem to allow those functions to 
complete after processing a limited number of events (chosen so as to not 
overflow the mieq queue), or if they notice that the event queue has crossed 
some high-water mark threshold.


On 05/02/2010 00:45, Mark Lillibridge wrote:
>      Given there doesn't seem to be anyone competent enough to attempt a
> more ambitious patch, I think we should apply the temporary patch.  How
> should we/I. go about doing this?  The Cygwin X website and contributors
> documentation doesn't actually say how to do this.
>
>     The patch is enclosed at the end.
>
> - Thanks,
>    Mark
>
>
> $ diff -u mieq.c~ mieq.c
> --- mieq.c~     2009-10-15 21:38:27.000000000 -0700
> +++ mieq.c      2010-02-04 16:42:30.773405200 -0800
> @@ -58,7 +58,7 @@
>   # include<X11/extensions/dpmsconst.h>
>   #endif
>
> -#define QUEUE_SIZE  512
> +#define QUEUE_SIZE  25000
>
>   #define EnqueueScreen(dev) dev->spriteInfo->sprite->pEnqueueScreen
>   #define DequeueScreen(dev) dev->spriteInfo->sprite->pDequeueScreen
>

Thanks for the patch.  Have you actually tested that this resolves your problem?

I'll add some words about contributing patches to the CG guide documentation, 
thanks for pointing out this oversight.

Perhaps that's the reason we hardly ever get any :-)

[1] 
http://x.cygwin.com/docs/cg/prog-server-architecture.html#prog-server-architecture-input

--
Jon TURNEY
Volunteer Cygwin/X X Server maintainer

--
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple
Problem reports:       http://cygwin.com/problems.html
Documentation:         http://x.cygwin.com/docs/
FAQ:                   http://x.cygwin.com/docs/faq/