From e01c3ab9fb93de2ebed0d3b8d146295491c76c63 Mon Sep 17 00:00:00 2001 From: "donal.k.fellows@manchester.ac.uk" Date: Sat, 2 Jan 2010 11:07:55 +0000 Subject: Fix [Bug 1373712] and [Bug 1924761]. --- ChangeLog | 19 ++++++++ carbon/tkMacOSXKeyEvent.c | 3 +- doc/HandleEvent.3 | 5 ++- doc/QWinEvent.3 | 5 ++- generic/tkEvent.c | 74 ++++++++++++++++++++++++------- generic/tkInt.h | 18 +++++++- macosx/tkMacOSXKeyEvent.c | 3 +- unix/tkUnixEvent.c | 35 ++++++++++++--- unix/tkUnixKey.c | 109 ++++++++++++++++++++++++---------------------- win/tkWinX.c | 3 +- 10 files changed, 194 insertions(+), 80 deletions(-) diff --git a/ChangeLog b/ChangeLog index 0bcdd4e..3af8fe4 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,22 @@ +2010-01-02 Donal K. Fellows + + * unix/tkUnixEvent.c (TransferXEventsToTcl): [Bug 1924761]: Use the + new cache mechanism to force the extraction of the string of a key + event from XIM at the right time rather than after queueing when it + can be quashed by a race condition centered on the limited amount of + state in some XIM implementations. + + * unix/tkUnixKey.c (TkpGetString): [Bug 1373712]: Cache the value that + * generic/tkInt.h (TkKeyEvent): will be substituted via %A so + * generic/tkEvent.c (CleanUpTkEvent): that we do not need to make it + * doc/HandleEvent.3 (ARGUMENTS): fresh each time, which causes + * doc/QWinEvent.3 (ARGUMENTS): trouble with some input + * carbon/tkMacOSXKeyEvent.c (InitKeyEvent): methods. Also includes the + * macosx/tkMacOSXKeyEvent.c (tkProcessKeyEvent): factoring out of some + * win/tkWinX.c (GenerateXEvent): code and update of + documentation to describe the slightly increased constraints on + how Tk_HandleEvent can be used. + 2010-01-01 Donal K. Fellows * unix/tkUnixEvent.c (TransferXEventsToTcl): [Bug 1924761]: Move the diff --git a/carbon/tkMacOSXKeyEvent.c b/carbon/tkMacOSXKeyEvent.c index 5bce60d..5ffdbd5 100644 --- a/carbon/tkMacOSXKeyEvent.c +++ b/carbon/tkMacOSXKeyEvent.c @@ -48,7 +48,7 @@ * permission to use and distribute the software in accordance with the * terms specified in this license. * - * RCS: @(#) $Id: tkMacOSXKeyEvent.c,v 1.1 2009/06/26 01:42:47 das Exp $ + * RCS: @(#) $Id: tkMacOSXKeyEvent.c,v 1.2 2010/01/02 11:07:55 dkf Exp $ */ #include "tkMacOSXPrivate.h" @@ -489,6 +489,7 @@ InitKeyEvent( return -1; } + memset(eventPtr, 0, sizeof(XEvent)); eventPtr->xany.send_event = false; eventPtr->xany.serial = Tk_Display(tkwin)->request; diff --git a/doc/HandleEvent.3 b/doc/HandleEvent.3 index 8f1b319..f313907 100644 --- a/doc/HandleEvent.3 +++ b/doc/HandleEvent.3 @@ -5,7 +5,7 @@ '\" See the file "license.terms" for information on usage and redistribution '\" of this file, and for a DISCLAIMER OF ALL WARRANTIES. '\" -'\" RCS: @(#) $Id: HandleEvent.3,v 1.6 2008/07/08 22:40:51 patthoyts Exp $ +'\" RCS: @(#) $Id: HandleEvent.3,v 1.7 2010/01/02 11:07:56 dkf Exp $ '\" .so man.macros .TH Tk_HandleEvent 3 "" Tk "Tk Library Procedures" @@ -20,7 +20,8 @@ Tk_HandleEvent \- invoke event handlers for window system events .SH ARGUMENTS .AS XEvent *eventPtr .AP XEvent *eventPtr in -Pointer to X event to dispatch to relevant handler(s). +Pointer to X event to dispatch to relevant handler(s). It is important +that all unused fields of the structure be set to zero. .BE .SH DESCRIPTION .PP diff --git a/doc/QWinEvent.3 b/doc/QWinEvent.3 index 0f82210..7066d49 100644 --- a/doc/QWinEvent.3 +++ b/doc/QWinEvent.3 @@ -4,7 +4,7 @@ '\" See the file "license.terms" for information on usage and redistribution '\" of this file, and for a DISCLAIMER OF ALL WARRANTIES. '\" -'\" RCS: @(#) $Id: QWinEvent.3,v 1.5 2008/07/08 22:40:51 patthoyts Exp $ +'\" RCS: @(#) $Id: QWinEvent.3,v 1.6 2010/01/02 11:07:56 dkf Exp $ '\" .so man.macros .TH Tk_QueueWindowEvent 3 7.5 Tk "Tk Library Procedures" @@ -26,7 +26,8 @@ Display for which to control motion event collapsing. .AP int collapse in Indicates whether motion events should be collapsed or not. .AP XEvent *eventPtr in -An event to add to the event queue. +An event to add to the event queue. It is important +that all unused fields of the structure be set to zero. .AP Tcl_QueuePosition position in Where to add the new event in the queue: \fBTCL_QUEUE_TAIL\fR, \fBTCL_QUEUE_HEAD\fR, or \fBTCL_QUEUE_MARK\fR. diff --git a/generic/tkEvent.c b/generic/tkEvent.c index 36f1c3a..fd5c129 100644 --- a/generic/tkEvent.c +++ b/generic/tkEvent.c @@ -12,7 +12,7 @@ * See the file "license.terms" for information on usage and redistribution of * this file, and for a DISCLAIMER OF ALL WARRANTIES. * - * RCS: @(#) $Id: tkEvent.c,v 1.40 2010/01/01 22:50:27 dkf Exp $ + * RCS: @(#) $Id: tkEvent.c,v 1.41 2010/01/02 11:07:56 dkf Exp $ */ #include "tkInt.h" @@ -193,6 +193,7 @@ TCL_DECLARE_MUTEX(exitMutex) * Prototypes for functions that are only referenced locally within this file. */ +static void CleanUpTkEvent(XEvent *eventPtr); static void DelayedMotionProc(ClientData clientData); static int GetButtonMask(unsigned int Button); static unsigned long GetEventMaskFromXEvent(XEvent *eventPtr); @@ -1229,7 +1230,7 @@ Tk_HandleEvent( */ if (InvokeGenericHandlers(tsdPtr, eventPtr)) { - goto releaseUserData; + goto releaseEventResources; } if (RefreshKeyboardMappingIfNeeded(eventPtr)) { @@ -1237,14 +1238,14 @@ Tk_HandleEvent( * We are done with a MappingNotify event. */ - goto releaseUserData; + goto releaseEventResources; } mask = GetEventMaskFromXEvent(eventPtr); winPtr = GetTkWindowFromXEvent(eventPtr); if (winPtr == NULL) { - goto releaseUserData; + goto releaseEventResources; } /* @@ -1258,7 +1259,7 @@ Tk_HandleEvent( if ((winPtr->flags & TK_ALREADY_DEAD) && (eventPtr->type != DestroyNotify)) { - goto releaseUserData; + goto releaseEventResources; } if (winPtr->mainPtr != NULL) { @@ -1382,18 +1383,11 @@ Tk_HandleEvent( * Release the user_data from the event (if it is a virtual event and the * field was non-NULL in the first place.) Note that this is done using a * Tcl_Obj interface, and we set the field back to NULL afterwards out of - * paranoia. + * paranoia. Also clean up any cached %A substitutions from key events. */ - releaseUserData: - if (eventPtr->type == VirtualEvent) { - XVirtualEvent *vePtr = (XVirtualEvent *) eventPtr; - - if (vePtr->user_data != NULL) { - Tcl_DecrRefCount(vePtr->user_data); - vePtr->user_data = NULL; - } - } + releaseEventResources: + CleanUpTkEvent(eventPtr); } /* @@ -1755,17 +1749,67 @@ WindowEventProc( * even though we didn't do anything at all. */ + CleanUpTkEvent(&wevPtr->event); return 1; } } } Tk_HandleEvent(&wevPtr->event); + CleanUpTkEvent(&wevPtr->event); return 1; } /* *---------------------------------------------------------------------- * + * CleanUpTkEvent -- + * + * This function is called to remove and deallocate any information in + * the event which is not directly in the event structure itself. It may + * be called multiple times per event, so it takes care to set the + * cleared pointer fields to NULL afterwards. + * + * Results: + * None. + * + * Side effects: + * Makes the event no longer have any external resources. + * + *---------------------------------------------------------------------- + */ + +static void +CleanUpTkEvent( + XEvent *eventPtr) +{ + switch (eventPtr->type) { + case KeyPress: + case KeyRelease: { + TkKeyEvent *kePtr = (TkKeyEvent *) eventPtr; + + if (kePtr->charValuePtr != NULL) { + ckfree(kePtr->charValuePtr); + kePtr->charValuePtr = NULL; + kePtr->charValueLen = 0; + } + break; + } + + case VirtualEvent: { + XVirtualEvent *vePtr = (XVirtualEvent *) eventPtr; + + if (vePtr->user_data != NULL) { + Tcl_DecrRefCount(vePtr->user_data); + vePtr->user_data = NULL; + } + break; + } + } +} + +/* + *---------------------------------------------------------------------- + * * DelayedMotionProc -- * * This function is invoked as an idle handler when a mouse motion event diff --git a/generic/tkInt.h b/generic/tkInt.h index a9062ec..73297d8 100644 --- a/generic/tkInt.h +++ b/generic/tkInt.h @@ -11,7 +11,7 @@ * See the file "license.terms" for information on usage and redistribution of * this file, and for a DISCLAIMER OF ALL WARRANTIES. * - * RCS: $Id: tkInt.h,v 1.116 2010/01/01 22:50:27 dkf Exp $ + * RCS: $Id: tkInt.h,v 1.117 2010/01/02 11:07:56 dkf Exp $ */ #ifndef _TKINT @@ -824,6 +824,22 @@ typedef struct TkWindow { } TkWindow; /* + * Real definition of some events. Note that these events come from outside + * but have internally generated pieces added to them. + */ + +typedef struct { + XKeyEvent keyEvent; /* The real event from X11. */ + char *charValuePtr; /* A pointer to a string that holds the key's + * %A substitution text (before backslash + * adding), or NULL if that has not been + * computed yet. If non-NULL, this string was + * allocated with ckalloc(). */ + int charValueLen; /* Length of string in charValuePtr when that + * is non-NULL. */ +} TkKeyEvent; + +/* * Flags passed to TkpMakeMenuWindow's 'transient' argument. */ diff --git a/macosx/tkMacOSXKeyEvent.c b/macosx/tkMacOSXKeyEvent.c index 6d4a367..4e081a0 100644 --- a/macosx/tkMacOSXKeyEvent.c +++ b/macosx/tkMacOSXKeyEvent.c @@ -10,7 +10,7 @@ * See the file "license.terms" for information on usage and redistribution of * this file, and for a DISCLAIMER OF ALL WARRANTIES. * - * RCS: @(#) $Id: tkMacOSXKeyEvent.c,v 1.28 2009/07/06 20:29:21 dkf Exp $ + * RCS: @(#) $Id: tkMacOSXKeyEvent.c,v 1.29 2010/01/02 11:07:56 dkf Exp $ */ #include "tkMacOSXPrivate.h" @@ -107,6 +107,7 @@ static NSModalSession modalSession = NULL; XEvent xEvent; + memset(xEvent, 0, sizeof(XEvent)); xEvent.xany.serial = LastKnownRequestProcessed(Tk_Display(tkwin)); xEvent.xany.send_event = false; xEvent.xany.display = Tk_Display(tkwin); diff --git a/unix/tkUnixEvent.c b/unix/tkUnixEvent.c index 39e3e5e..4542876 100644 --- a/unix/tkUnixEvent.c +++ b/unix/tkUnixEvent.c @@ -9,7 +9,7 @@ * See the file "license.terms" for information on usage and redistribution of * this file, and for a DISCLAIMER OF ALL WARRANTIES. * - * RCS: @(#) $Id: tkUnixEvent.c,v 1.33 2010/01/01 22:50:27 dkf Exp $ + * RCS: @(#) $Id: tkUnixEvent.c,v 1.34 2010/01/02 11:07:56 dkf Exp $ */ #include "tkUnixInt.h" @@ -277,12 +277,15 @@ TransferXEventsToTcl( Display *display) { union { + int type; XEvent x; + TkKeyEvent k; #ifdef GenericEvent xGenericEvent xge; #endif } event; Window w; + TkDisplay *dispPtr = NULL; /* * Transfer events from the X event queue to the Tk event queue after XIM @@ -298,15 +301,13 @@ TransferXEventsToTcl( while (QLength(display) > 0) { XNextEvent(display, &event.x); #ifdef GenericEvent - if (event.x.type == GenericEvent) { + if (event.type == GenericEvent) { Tcl_Panic("Wild GenericEvent; panic! (extension=%d,evtype=%d)", event.xge.extension, event.xge.evtype); } #endif w = None; - if (event.x.type == KeyPress || event.x.type == KeyRelease) { - TkDisplay *dispPtr; - + if (event.type == KeyPress || event.type == KeyRelease) { for (dispPtr = TkGetDisplayList(); ; dispPtr = dispPtr->nextPtr) { if (dispPtr == NULL) { break; @@ -321,6 +322,30 @@ TransferXEventsToTcl( if (XFilterEvent(&event.x, w)) { continue; } + if (event.type == KeyPress || event.type == KeyRelease) { + event.k.charValuePtr = NULL; + event.k.charValueLen = 0; + + /* + * Force the calling of the input method engine now. The results + * from it will be cached in the event so that they don't get lost + * (to a race condition with other XIM-handled key events) between + * entering the event queue and getting serviced. [Bug 1924761] + */ + +#ifdef TK_USE_INPUT_METHODS + if (event.type == KeyPress && dispPtr && + (dispPtr->flags & TK_DISPLAY_USE_IM)) { + if (dispPtr->focusPtr && dispPtr->focusPtr->inputContext) { + Tcl_DString ds; + + Tcl_DStringInit(&ds); + (void) TkpGetString(dispPtr->focusPtr, &event.x, &ds); + Tcl_DStringFree(&ds); + } + } +#endif + } Tk_QueueWindowEvent(&event.x, TCL_QUEUE_TAIL); } } diff --git a/unix/tkUnixKey.c b/unix/tkUnixKey.c index 49ee3e0..ef0588f 100644 --- a/unix/tkUnixKey.c +++ b/unix/tkUnixKey.c @@ -9,7 +9,7 @@ * See the file "license.terms" for information on usage and redistribution of * this file, and for a DISCLAIMER OF ALL WARRANTIES. * - * RCS: @(#) $Id: tkUnixKey.c,v 1.16 2009/12/16 22:00:30 nijtmans Exp $ + * RCS: @(#) $Id: tkUnixKey.c,v 1.17 2010/01/02 11:07:56 dkf Exp $ */ #include "tkInt.h" @@ -76,50 +76,6 @@ Tk_SetCaretPos( /* *---------------------------------------------------------------------- * - * TkpGetChar -- - * - * Convert a keyboard event to a UTF-8 string using XLookupString. - * - * This is used as a fallback instead of Xutf8LookupString or - * XmbLookupString if input methods are turned off and for KeyRelease - * events. - * - * Notes: - * XLookupString() normally returns a single ISO Latin 1 or ASCII control - * character. - * - *---------------------------------------------------------------------- - */ -static const char * -TkpGetChar( - XEvent *eventPtr, /* KeyPress or KeyRelease event */ - Tcl_DString *dsPtr) /* Initialized, empty string to hold result. */ -{ - int len; - char buf[TCL_DSTRING_STATIC_SIZE]; - - len = XLookupString(&eventPtr->xkey, buf, TCL_DSTRING_STATIC_SIZE, 0, 0); - buf[len] = '\0'; - - if (len == 1) { - len = Tcl_UniCharToUtf((unsigned char) buf[0], - Tcl_DStringValue(dsPtr)); - Tcl_DStringSetLength(dsPtr, len); - } else { - /* - * len > 1 should only happen if someone has called XRebindKeysym(). - * Assume UTF-8. - */ - - Tcl_DStringSetLength(dsPtr, len); - strncpy(Tcl_DStringValue(dsPtr), buf, len); - } - return Tcl_DStringValue(dsPtr); -} - -/* - *---------------------------------------------------------------------- - * * TkpGetString -- * * Retrieve the UTF string associated with a keyboard event. @@ -141,11 +97,25 @@ TkpGetString( XEvent *eventPtr, /* X keyboard event. */ Tcl_DString *dsPtr) /* Initialized, empty string to hold result. */ { + int len; + Tcl_DString buf; + TkKeyEvent *kePtr = (TkKeyEvent *) eventPtr; + + /* + * If we have the value cached already, use it now. [Bug 1373712] + */ + + if (kePtr->charValuePtr != NULL) { + Tcl_DStringSetLength(dsPtr, kePtr->charValueLen); + memcpy(Tcl_DStringValue(dsPtr), kePtr->charValuePtr, + (unsigned) kePtr->charValueLen+1); + return Tcl_DStringValue(dsPtr); + } + #ifdef TK_USE_INPUT_METHODS if ((winPtr->dispPtr->flags & TK_DISPLAY_USE_IM) && (winPtr->inputContext != NULL) && (eventPtr->type == KeyPress)) { - int len; Status status; #if X_HAVE_UTF8_STRING @@ -165,13 +135,10 @@ TkpGetString( NULL, &status); } if ((status != XLookupChars) && (status != XLookupBoth)) { - Tcl_DStringSetLength(dsPtr, 0); len = 0; } Tcl_DStringSetLength(dsPtr, len); #else /* !X_HAVE_UTF8_STRING */ - Tcl_DString buf; /* Holds string in system encoding. */ - /* * Overallocate the dstring to the maximum stack amount. */ @@ -198,11 +165,49 @@ TkpGetString( Tcl_ExternalToUtfDString(NULL, Tcl_DStringValue(&buf), len, dsPtr); Tcl_DStringFree(&buf); #endif /* X_HAVE_UTF8_STRING */ + } else +#endif /* TK_USE_INPUT_METHODS */ + { + /* + * Fall back to convert a keyboard event to a UTF-8 string using + * XLookupString. This is used when input methods are turned off and + * for KeyRelease events. + * + * Note: XLookupString() normally returns a single ISO Latin 1 or + * ASCII control character. + */ - return Tcl_DStringValue(dsPtr); + Tcl_DStringInit(&buf); + Tcl_DStringSetLength(&buf, TCL_DSTRING_STATIC_SIZE-1); + len = XLookupString(&eventPtr->xkey, Tcl_DStringValue(&buf), + TCL_DSTRING_STATIC_SIZE, 0, 0); + Tcl_DStringValue(&buf)[len] = '\0'; + + if (len == 1) { + len = Tcl_UniCharToUtf((unsigned char) Tcl_DStringValue(&buf)[0], + Tcl_DStringValue(dsPtr)); + Tcl_DStringSetLength(dsPtr, len); + } else { + /* + * len > 1 should only happen if someone has called XRebindKeysym. + * Assume UTF-8. + */ + + Tcl_DStringSetLength(dsPtr, len); + strncpy(Tcl_DStringValue(dsPtr), Tcl_DStringValue(&buf), len); + } } -#endif /* TK_USE_INPUT_METHODS */ - return TkpGetChar(eventPtr, dsPtr); + + /* + * Cache the string in the event so that if/when we return to this + * function, we will be able to produce it without asking X. This stops us + * from having to reenter the XIM engine. [Bug 1373712] + */ + + kePtr->charValuePtr = ckalloc((unsigned) len + 1); + kePtr->charValueLen = len; + memcpy(kePtr->charValuePtr, Tcl_DStringValue(dsPtr), (unsigned) len + 1); + return Tcl_DStringValue(dsPtr); } /* diff --git a/win/tkWinX.c b/win/tkWinX.c index a91c3bd..8b27b1e 100644 --- a/win/tkWinX.c +++ b/win/tkWinX.c @@ -10,7 +10,7 @@ * See the file "license.terms" for information on usage and redistribution of * this file, and for a DISCLAIMER OF ALL WARRANTIES. * - * RCS: @(#) $Id: tkWinX.c,v 1.61 2009/08/02 21:40:17 nijtmans Exp $ + * RCS: @(#) $Id: tkWinX.c,v 1.62 2010/01/02 11:07:56 dkf Exp $ */ /* @@ -1033,6 +1033,7 @@ GenerateXEvent( return; } + memset(&event, 0, sizeof(XEvent)); event.xany.serial = winPtr->display->request++; event.xany.send_event = False; event.xany.display = winPtr->display; -- cgit v0.12