Re: bug report/suggested temp. patch: handling bursts of sent keys
On 08/04/2010 04:47, Mark Lillibridge wrote: Can you pull this patch and include it in the next X server release, please. Excellent. Thank you. Do we need to do anything special for this to get picked up for Xming as well? Assuming I don't receive too much hatemail about this breaking stuff after the next X server release, I'll push it upstream to X.Org. Thanks! Any idea when the next X server release is? - Mark -- 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/
Re: bug report/suggested temp. patch: handling bursts of sent keys
On 08/04/2010 04:47, Mark Lillibridge wrote: Can you pull this patch and include it in the next X server release, please. Excellent. Thank you. Do we need to do anything special for this to get picked up for Xming as well? Assuming I don't receive too much hatemail about this breaking stuff after the next X server release, I'll push it upstream to X.Org. Colin Harrison, the Xming maintainer, periodically syncs from X.Org, but it's his decision what he includes. -- 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/
Re: bug report/suggested temp. patch: handling bursts of sent keys
Jon wrote: Yaakov, Can you pull this patch and include it in the next X server release, please. Excellent. Thank you. Do we need to do anything special for this to get picked up for Xming as well? - Mark -- 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/
Re: bug report/suggested temp. patch: handling bursts of sent keys
On 20/03/2010 16:32, Mark Lillibridge wrote: On February 23, 2010, Jon Turney wrote: On 13/02/2010 20:24, Mark Lillibridge wrote: Jon wrote: Thanks for the patch. Have you actually tested that this resolves your problem? Yes. Of course, really, really large bursts will still fail, but they should be very rare. Perhaps you might try the attached patch, instead, and see if that helps. Yes, your (previously) attached patch seems to solve the problem without wasting memory or only working for a finite number of keys. Nice job. What's the next step towards getting this patch added to the source/sent upstream/to Xming? Yaakov, Can you pull this patch and include it in the next X server release, please. Sadly this isn't quite a complete solution: there's still the problem in multiwindow mode that Windows uses a modal loop when resizing/moving a window, so the X server code gets no chance to do anything then. You can easily demonstrate this by resizing the frame of an X window, moving the mouse continuously for a number of seconds, and then notice that X window doesn't react to the queued size changes until the mouse button is released. The following changes since commit 579715f830fbbca9e1ecb17dc18176132f5969e7: Rami Ylimaki (1): os: Prevent backtrace from being stopped in noreturn functions. are available in the git repository at: git://anongit.freedesktop.org/~jturney/xserver for-yaakov Jon TURNEY (1): Cygwin/X: Process one Windows message per wakeup, rather than all of them. hw/xwin/winblock.c | 23 --- hw/xwin/winwakeup.c |4 ++-- 2 files changed, 6 insertions(+), 21 deletions(-) -- 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/
Re: bug report/suggested temp. patch: handling bursts of sent keys
On February 23, 2010, Jon Turney wrote: On 13/02/2010 20:24, Mark Lillibridge wrote: Jon wrote: Thanks for the patch. Have you actually tested that this resolves your problem? Yes. Of course, really, really large bursts will still fail, but they should be very rare. Perhaps you might try the attached patch, instead, and see if that helps. Yes, your (previously) attached patch seems to solve the problem without wasting memory or only working for a finite number of keys. Nice job. What's the next step towards getting this patch added to the source/sent upstream/to Xming? - Thanks, Mark -- 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/
Re: bug report/suggested temp. patch: handling bursts of sent keys
On 13/02/2010 20:24, Mark Lillibridge wrote: Jon wrote: Thanks for the patch. Have you actually tested that this resolves your problem? Yes. Of course, really, really large bursts will still fail, but they should be very rare. Perhaps you might try the attached patch, instead, and see if that helps. 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. This is an interesting idea; I spent a while looking at WaitFor.c/WaitForSomething, but it's pretty opaque -- couldn't figure out when/under what conditions winWakeHandler is called by it. E.g., what's the actual priority between emptying the queue and processing window messages? Let's see: right at the end of hw/xwin/InitInput.c we open /dev/windows (a special device which becomes ready when there is anything on the windows message queue) and add that to the select mask. When the select returns, all the wakeup handlers are run, including winWakeHandler() which checks the WM message queue. -- Jon TURNEY Volunteer Cygwin/X X Server maintainer 0001-Process-one-Windows-message-per-wakeup-rather-than-a.patch Description: application/itunes-itlp -- 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/
Re: bug report/suggested temp. patch: handling bursts of sent keys
Jon wrote: Thanks for the patch. Have you actually tested that this resolves your problem? Yes. Of course, really, really large bursts will still fail, but they should be very rare. 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 :-) You think? :-) 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. This is an interesting idea; I spent a while looking at WaitFor.c/WaitForSomething, but it's pretty opaque -- couldn't figure out when/under what conditions winWakeHandler is called by it. E.g., what's the actual priority between emptying the queue and processing window messages? - Mark -- 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/
Re: bug report/suggested temp. patch: handling bursts of sent keys
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.0 -0700 +++ mieq.c 2010-02-04 16:42:30.773405200 -0800 @@ -58,7 +58,7 @@ # includeX11/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/
Re: bug report/suggested temp. patch: handling bursts of sent keys
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.0 -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 -- 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/
Re: bug report/suggested temp. patch: handling bursts of sent keys
I've done a bit more investigating. It appears that winWindowProc can block at the cost of preventing processing of any Windows messages until it returns. Does anyone know how fast the mieq queue drains? If this isn't very fast, then having winWindowProc block waiting for the mieq queue is probably a bad idea. There are a number of comments in mieq.c that may shed light on why the queue is statically allocated: mieq.c:76: EventRec events[QUEUE_SIZE]; /* static allocation for signals */ mieq.c:138: /* * Must be reentrant with ProcessInputEvents. Assumption: mieqEnqueue * will never be interrupted. If this is called from both signal * handlers and regular code, make sure the signal is suspended when * called from regular code. */ void mieqEnqueue(DeviceIntPtr pDev, InternalEvent *e) Does anyone understand this? Does this bar dynamically allocating the queue? The queue does not appear to use locking unless XQUARTZ is defined. Does anyone know what conditions that is defined under? I'm beginning to think that simply patching the queue to be very large is the best solution. - Mark -- 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/
Re: bug report/suggested temp. patch: handling bursts of sent keys
On 2010-02-01 12:33 AM, Mark Lillibridge wrote: I've done a bit more investigating. It appears that winWindowProc can block at the cost of preventing processing of any Windows messages until it returns. Does anyone know how fast the mieq queue drains? If this isn't very fast, then having winWindowProc block waiting for the mieq queue is probably a bad idea. There are a number of comments in mieq.c that may shed light on why the queue is statically allocated: mieq.c:76: EventRec events[QUEUE_SIZE]; /* static allocation for signals */ mieq.c:138: /* * Must be reentrant with ProcessInputEvents. Assumption: mieqEnqueue * will never be interrupted. If this is called from both signal * handlers and regular code, make sure the signal is suspended when * called from regular code. */ void mieqEnqueue(DeviceIntPtr pDev, InternalEvent *e) Does anyone understand this? Does this bar dynamically allocating the queue? The queue does not appear to use locking unless XQUARTZ is defined. Does anyone know what conditions that is defined under? I'm beginning to think that simply patching the queue to be very large is the best solution. - Mark Well XQuartz is what the server is called on Mac OS X -- 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/
Re: bug report/suggested temp. patch: handling bursts of sent keys
Dennis wrote: Hi Mark. I am a bit new to this list, but not THAT new to programming. If you don't know how many keystrokes you need to have a buffer for, then you have 2 choices: 1. Have a ridiculously huge buffer. 2. Setup a dynamic array. Using a 25000 character buffer seems like overkill. But mieq.c is probably reading just from the Operating Systems' keyboard buffer. You may need to write an input function to feed mieq.c and somehow link to it. Sadly, that level of coding is beyond my abilities. Option 1 would be a temporary patch. Browsing through the source code, keypresses/releases come one at a time in via window messages to the following routine in hw/xwin/winwndproc.c: /* * Called by winWakeupHandler * Processes current Windows message */ LRESULT CALLBACK winWindowProc (HWND hwnd, UINT message, WPARAM wParam, LPARAM lParam) { ... case WM_SYSKEYDOWN: case WM_KEYDOWN: if (s_pScreenPriv == NULL || s_pScreenInfo-fIgnoreInput) break; ... /* Translate Windows key code to X scan code */ winTranslateKey (wParam, lParam, iScanCode); /* Ignore repeats for CapsLock */ if (wParam == VK_CAPITAL) lParam = 1; /* Send the key event(s) */ for (i = 0; i LOWORD(lParam); ++i) winSendKeyEvent (iScanCode, TRUE); return 0; winSendKeyEvent in turn lives in hw/xwin/winkeybd.c: /* * Take a raw X key code and send an up or down event for it. * * Thanks to VNC for inspiration, though it is a simple function. */ void winSendKeyEvent (DWORD dwKey, Bool fDown) { EventListPtr events; int i, nevents; /* * When alt-tabing between screens we can get phantom key up messages * Here we only pass them through it we think we should! */ if (g_winKeyState[dwKey] == FALSE fDown == FALSE) return; /* Update the keyState map */ g_winKeyState[dwKey] = fDown; GetEventList(events); nevents = GetKeyboardEvents(events, g_pwinKeyboard, fDown ? KeyPress : KeyRele ase, dwKey + MIN_KEYCODE); for (i = 0; i nevents; i++) mieqEnqueue(g_pwinKeyboard, events[i].event); #if CYGDEBUG ErrorF(winSendKeyEvent: dwKey: %d, fDown: %d, nEvents %d\n, dwKey, fDown, nevents); #endif } Note the call to mieqEnqueue there. 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? - Mark -- 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/
Re: bug report/suggested temp. patch: handling bursts of sent keys
Larry Hall wrote: On 01/19/2010 01:17 PM, Mark Lillibridge wrote: Hi. I don't appear to have gotten any response to my message sent to this list January 12 (copied below). Do I have the right list? Am I supposed to use a different mechanism to report bugs with the Cygwin X server? Please help. Thanks for the information you've provided. This is the correct list for Cygwin X issues. I can't engage you on this topic because I'm not knowledgeable about the area you investigated. However, a couple of questions come to my mind about what you've found: 1. Is this Cygwin-specific? The Xming server, which I understand shares source code, also has this bug. The commercial X server, Reflection, does not have this bug. This bug is specific to Windows implementations so I presume it does not belong in the generic Xorg source. 2. If not, what's the upstream solution? This information might help you decide if your issue is better reported upstream. It may also help developers here decide how to solve the problem. -- 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/
Re: bug report/suggested temp. patch: handling bursts of sent keys
Hi. I don't appear to have gotten any response to my message sent to this list January 12 (copied below). Do I have the right list? Am I supposed to use a different mechanism to report bugs with the Cygwin X server? Please help. - Thanks, Mark 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. Problem/temporary patch: I investigated and found that the problem appears to be that the X event queue (miEventQueue) in hw/mi/mieq.c is statically allocated with a ridiculously small value (512). When keyboard bursts come in, this queue overflows, causing the problem. If I set this queue to a more reasonable value of 5120: mieq.c:62:#define QUEUE_SIZE 5120/* was 512 */ then 10 times larger key bursts can be accommodated without problem. This value is probably still too small in practice so it would be safer to go with a larger value like 25000. I characterize this as a temporary patch because ideally either the queue would be made dynamic with no upper bound in size, or some kind of flow control would be implemented so the code that receives Windows key events does not overflow the event queue. Reproducing the problem: Dragon NaturallySpeaking costs money, so I will instead describe how to demonstrate the problem using AutoHotkey, which is a free download from http://www.autohotkey.com/. Download that program then create and run the following script, burst.ahk: cut here for burst.ahk #space:: Send ** ** ** ** **{enter}** ** ** ** **{enter}** ** ** ** **{enter}** ** ** ** **{enter}** ** ** ** **{enter} = Finally, type the Windows key and space together while focus is on an X application. The bug is not present, the following will be typed: ** ** ** ** ** ** ** ** ** ** ** ** ** ** ** ** ** ** ** ** ** ** ** ** ** On the other hand, if the bug is present you'll get something more like: ** ** ** ** ** ** ** ** ** ** *** - Mark -- 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/
Re: bug report/suggested temp. patch: handling bursts of sent keys
On 01/19/2010 01:17 PM, Mark Lillibridge wrote: Hi. I don't appear to have gotten any response to my message sent to this list January 12 (copied below). Do I have the right list? Am I supposed to use a different mechanism to report bugs with the Cygwin X server? Please help. Thanks for the information you've provided. This is the correct list for Cygwin X issues. I can't engage you on this topic because I'm not knowledgeable about the area you investigated. However, a couple of questions come to my mind about what you've found: 1. Is this Cygwin-specific? 2. If not, what's the upstream solution? This information might help you decide if your issue is better reported upstream. It may also help developers here decide how to solve the problem. -- Larry Hall http://www.rfk.com RFK Partners, Inc. (508) 893-9779 - RFK Office 216 Dalton Rd. (508) 893-9889 - FAX Holliston, MA 01746 _ A: Yes. Q: Are you sure? A: Because it reverses the logical flow of conversation. Q: Why is top posting annoying in email? -- 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/
bug report/suggested temp. patch: handling bursts of sent keys
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. Problem/temporary patch: I investigated and found that the problem appears to be that the X event queue (miEventQueue) in hw/mi/mieq.c is statically allocated with a ridiculously small value (512). When keyboard bursts come in, this queue overflows, causing the problem. If I set this queue to a more reasonable value of 5120: mieq.c:62:#define QUEUE_SIZE 5120/* was 512 */ then 10 times larger key bursts can be accommodated without problem. This value is probably still too small in practice so it would be safer to go with a larger value like 25000. I characterize this as a temporary patch because ideally either the queue would be made dynamic with no upper bound in size, or some kind of flow control would be implemented so the code that receives Windows key events does not overflow the event queue. Reproducing the problem: Dragon NaturallySpeaking costs money, so I will instead describe how to demonstrate the problem using AutoHotkey, which is a free download from http://www.autohotkey.com/. Download that program then create and run the following script, burst.ahk: cut here for burst.ahk #space:: Send ** ** ** ** **{enter}** ** ** ** **{enter}** ** ** ** **{enter}** ** ** ** **{enter}** ** ** ** **{enter} = Finally, type the Windows key and space together while focus is on an X application. The bug is not present, the following will be typed: ** ** ** ** ** ** ** ** ** ** ** ** ** ** ** ** ** ** ** ** ** ** ** ** ** On the other hand, if the bug is present you'll get something more like: ** ** ** ** ** ** ** ** ** ** *** - Mark -- 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/