From 184d44a35ac47f0676d36ba9f8d0d27a6dae0e5a Mon Sep 17 00:00:00 2001 From: "donal.k.fellows@manchester.ac.uk" Date: Sat, 2 Jan 2010 10:43:25 +0000 Subject: Fix [Bug 1373712] and [Bug 1924761]. --- ChangeLog | 18 ++++++ doc/HandleEvent.3 | 7 ++- doc/QWinEvent.3 | 5 +- generic/tkEvent.c | 74 ++++++++++++++++++----- generic/tkInt.h | 23 ++++++-- macosx/tkMacOSXKeyEvent.c | 3 +- unix/tkUnixEvent.c | 43 +++++++++++--- unix/tkUnixKey.c | 147 +++++++++++++++++++++------------------------- win/tkWinX.c | 3 +- 9 files changed, 209 insertions(+), 114 deletions(-) diff --git a/ChangeLog b/ChangeLog index dd9c9de..e15dc71 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,21 @@ +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 + * macosx/tkMacOSXKeyEvent.c (InitKeyEvent): methods. Also includes the + * win/tkWinX.c (GenerateXEvent): factoring out of some 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/doc/HandleEvent.3 b/doc/HandleEvent.3 index ea461d5..6067e6e 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.4 1999/04/21 21:53:22 rjohnson Exp $ +'\" RCS: @(#) $Id: HandleEvent.3,v 1.4.28.1 2010/01/02 10:43:26 dkf Exp $ '\" .so man.macros .TH Tk_HandleEvent 3 "" Tk "Tk Library Procedures" @@ -20,14 +20,15 @@ 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 \fBTk_HandleEvent\fR is a lower-level procedure that deals with window events. It is called by \fBTcl_ServiceEvent\fR (and indirectly by -\fBTk_DoOneEvent\fR), and in a few other cases within Tk. +\fBTcl_DoOneEvent\fR), and in a few other cases within Tk. It makes callbacks to any window event handlers (created by calls to \fBTk_CreateEventHandler\fR) that match \fIeventPtr\fR and then returns. In some cases diff --git a/doc/QWinEvent.3 b/doc/QWinEvent.3 index 15492dc..29efa49 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.3 2002/06/15 00:21:42 hobbs Exp $ +'\" RCS: @(#) $Id: QWinEvent.3,v 1.3.10.1 2010/01/02 10:43:26 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 d298f46..cd129a0 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.35.2.3 2010/01/01 23:03:42 dkf Exp $ + * RCS: @(#) $Id: tkEvent.c,v 1.35.2.4 2010/01/02 10:43:26 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); @@ -1235,7 +1236,7 @@ Tk_HandleEvent( */ if (InvokeGenericHandlers(tsdPtr, eventPtr)) { - goto releaseUserData; + goto releaseEventResources; } if (RefreshKeyboardMappingIfNeeded(eventPtr)) { @@ -1243,14 +1244,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; } /* @@ -1264,7 +1265,7 @@ Tk_HandleEvent( if ((winPtr->flags & TK_ALREADY_DEAD) && (eventPtr->type != DestroyNotify)) { - goto releaseUserData; + goto releaseEventResources; } if (winPtr->mainPtr != NULL) { @@ -1388,18 +1389,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); } /* @@ -1761,17 +1755,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 9cec9ab..cb6c7d9 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.82.2.5 2010/01/01 23:03:42 dkf Exp $ + * RCS: $Id: tkInt.h,v 1.82.2.6 2010/01/02 10:43:26 dkf Exp $ */ #ifndef _TKINT @@ -853,6 +853,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; + +/* * The following structure is used as a two way map between integers and * strings, usually to map between an internal C representation and the * strings used in Tcl. @@ -1113,9 +1129,8 @@ MODULE_SCOPE int Tk_WmObjCmd(ClientData clientData, Tcl_Interp *interp, int objc, Tcl_Obj *const objv[]); MODULE_SCOPE int Tk_GetDoublePixelsFromObj(Tcl_Interp *interp, - Tk_Window tkwin, - Tcl_Obj *objPtr, - double *doublePtr); + Tk_Window tkwin, Tcl_Obj *objPtr, + double *doublePtr); MODULE_SCOPE void TkEventInit(void); MODULE_SCOPE void TkRegisterObjTypes(void); diff --git a/macosx/tkMacOSXKeyEvent.c b/macosx/tkMacOSXKeyEvent.c index 35eef36..b27998f 100644 --- a/macosx/tkMacOSXKeyEvent.c +++ b/macosx/tkMacOSXKeyEvent.c @@ -54,7 +54,7 @@ * software in accordance with the terms specified in this * license. * - * RCS: @(#) $Id: tkMacOSXKeyEvent.c,v 1.24.2.1 2008/09/02 16:14:18 das Exp $ + * RCS: @(#) $Id: tkMacOSXKeyEvent.c,v 1.24.2.2 2010/01/02 10:43:26 dkf Exp $ */ #include "tkMacOSXPrivate.h" @@ -496,6 +496,7 @@ InitKeyEvent( return -1; } + memset(eventPtr, 0, sizeof(XEvent)); eventPtr->xany.send_event = false; eventPtr->xany.serial = Tk_Display(tkwin)->request; diff --git a/unix/tkUnixEvent.c b/unix/tkUnixEvent.c index a104aad..16ab4f7 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.27.2.2 2010/01/02 00:00:20 dkf Exp $ + * RCS: @(#) $Id: tkUnixEvent.c,v 1.27.2.3 2010/01/02 10:43:26 dkf Exp $ */ #include "tkUnixInt.h" @@ -278,8 +278,13 @@ static void TransferXEventsToTcl( Display *display) { - XEvent event; + union { + int type; + XEvent x; + TkKeyEvent k; + } event; Window w; + TkDisplay *dispPtr = NULL; /* * Transfer events from the X event queue to the Tk event queue after XIM @@ -293,15 +298,13 @@ TransferXEventsToTcl( */ while (QLength(display) > 0) { - XNextEvent(display, &event); + XNextEvent(display, &event.x); w = None; if (event.type == KeyPress || event.type == KeyRelease) { - TkDisplay *dispPtr; - for (dispPtr = TkGetDisplayList(); ; dispPtr = dispPtr->nextPtr) { if (dispPtr == NULL) { break; - } else if (dispPtr->display == event.xany.display) { + } else if (dispPtr->display == event.x.xany.display) { if (dispPtr->focusPtr != NULL) { w = dispPtr->focusPtr->window; } @@ -309,10 +312,34 @@ TransferXEventsToTcl( } } } - if (XFilterEvent(&event, w)) { + if (XFilterEvent(&event.x, w)) { continue; } - Tk_QueueWindowEvent(&event, TCL_QUEUE_TAIL); + 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 2fb8ebc..d482b16 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.13.2.1 2008/06/11 00:41:25 jenglish Exp $ + * RCS: @(#) $Id: tkUnixKey.c,v 1.13.2.2 2010/01/02 10:43:26 dkf Exp $ */ #include "tkInt.h" @@ -78,48 +78,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 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. @@ -140,16 +98,30 @@ TkpGetString( TkWindow *winPtr, /* Window where event occurred */ XEvent *eventPtr, /* X keyboard event. */ Tcl_DString *dsPtr) /* Initialized, empty string to hold result. */ -#ifdef TK_USE_INPUT_METHODS -#if X_HAVE_UTF8_STRING { + 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 Tcl_DStringSetLength(dsPtr, TCL_DSTRING_STATIC_SIZE-1); len = Xutf8LookupString(winPtr->inputContext, &eventPtr->xkey, Tcl_DStringValue(dsPtr), Tcl_DStringLength(dsPtr), @@ -162,32 +134,16 @@ TkpGetString( NULL, &status); } if ((status != XLookupChars) && (status != XLookupBoth)) { - Tcl_DStringSetLength(dsPtr, 0); len = 0; } Tcl_DStringSetLength(dsPtr, len); - - return Tcl_DStringValue(dsPtr); - } else { - return TkpGetChar(eventPtr, dsPtr); - } -} #else /* !X_HAVE_UTF8_STRING */ -{ - int len; - Tcl_DString buf; - Status status; - - /* - * Overallocate the dstring to the maximum stack amount. - */ - - Tcl_DStringInit(&buf); - Tcl_DStringSetLength(&buf, TCL_DSTRING_STATIC_SIZE-1); + /* + * Overallocate the dstring to the maximum stack amount. + */ - if ((winPtr->dispPtr->flags & TK_DISPLAY_USE_IM) - && (winPtr->inputContext != NULL) - && (eventPtr->type == KeyPress)) { + Tcl_DStringInit(&buf); + Tcl_DStringSetLength(&buf, TCL_DSTRING_STATIC_SIZE-1); len = XmbLookupString(winPtr->inputContext, &eventPtr->xkey, Tcl_DStringValue(&buf), Tcl_DStringLength(&buf), NULL, @@ -205,24 +161,55 @@ TkpGetString( if ((status != XLookupChars) && (status != XLookupBoth)) { len = 0; } - } else { - return TkpGetChar(eventPtr, dsPtr); + + Tcl_DStringSetLength(&buf, len); + 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. + */ + + 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); + } } - Tcl_DStringSetLength(&buf, len); - Tcl_ExternalToUtfDString(NULL, Tcl_DStringValue(&buf), len, dsPtr); - Tcl_DStringFree(&buf); + /* + * 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); } - -#endif /* X_HAVE_UTF8_STRING */ -#else /* !TK_USE_INPUT_METHODS */ -{ - return TkpGetChar(eventPtr, dsPtr); -} -#endif - /* * When mapping from a keysym to a keycode, need information about the diff --git a/win/tkWinX.c b/win/tkWinX.c index e5e9e7b..e603e4a 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.57 2007/12/13 15:28:56 dgp Exp $ + * RCS: @(#) $Id: tkWinX.c,v 1.57.2.1 2010/01/02 10:43:26 dkf Exp $ */ /* @@ -1010,6 +1010,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