Fix [Bug 1373712] and [Bug 1924761].

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  <dkf@users.sf.net>
+
+	* 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  <dkf@users.sf.net>
 
 	* 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;