Backport fix for [Bug 1114977]; canvas tag searches now work with threads.

2 files changed, 151 insertions, 81 deletions

diff --git a/ChangeLog b/ChangeLog
index aa989d2..c6c1733 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@
+2005-06-04  Donal K. Fellows  <donal.k.fellows@man.ac.uk>
+
+	* generic/tkCanvas.c (SearchUids): Move all global data into
+	protected storage to stop cross-thread issues. [Bug 1114977] 
+
 2005-06-03  Donal K. Fellows  <donal.k.fellows@man.ac.uk>
 
 	* generic/tkConsole.c (Tk_CreateConsoleWindow, TkConsolePrint):
diff --git a/generic/tkCanvas.c b/generic/tkCanvas.c
index 427eb22..a97ba38 100644
--- a/generic/tkCanvas.c
+++ b/generic/tkCanvas.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: tkCanvas.c,v 1.21 2003/02/09 07:48:22 hobbs Exp $
+ * RCS: @(#) $Id: tkCanvas.c,v 1.21.2.1 2005/06/03 23:22:40 dkf Exp $
  */
 
 /* #define USE_OLD_TAG_SEARCH 1 */
@@ -205,27 +205,34 @@ static Tk_ConfigSpec configSpecs[] = {
 };
 
 /*
- * List of all the item types known at present:
+ * List of all the item types known at present. This is *global* and
+ * is protected by typeListMutex.
  */
 
 static Tk_ItemType *typeList = NULL;	/* NULL means initialization hasn't
 					 * been done yet. */
+TCL_DECLARE_MUTEX(typeListMutex);
 
 #ifndef USE_OLD_TAG_SEARCH
 /*
  * Uids for operands in compiled advanced tag search expressions
- * Initialization is done by InitCanvas()
+ * Initialization is done by GetStaticUids()
  */
-static Tk_Uid allUid = NULL;
-static Tk_Uid currentUid = NULL;
-static Tk_Uid andUid = NULL;
-static Tk_Uid orUid = NULL;
-static Tk_Uid xorUid = NULL;
-static Tk_Uid parenUid = NULL;
-static Tk_Uid negparenUid = NULL;
-static Tk_Uid endparenUid = NULL;
-static Tk_Uid tagvalUid = NULL;
-static Tk_Uid negtagvalUid = NULL;
+typedef struct {
+    Tk_Uid allUid;
+    Tk_Uid currentUid;
+    Tk_Uid andUid;
+    Tk_Uid orUid;
+    Tk_Uid xorUid;
+    Tk_Uid parenUid;
+    Tk_Uid negparenUid;
+    Tk_Uid endparenUid;
+    Tk_Uid tagvalUid;
+    Tk_Uid negtagvalUid;
+} SearchUids;
+
+static Tcl_ThreadDataKey dataKey;
+static SearchUids *GetStaticUids _ANSI_ARGS_((void));
 #endif /* USE_OLD_TAG_SEARCH */
 
 /*
@@ -928,11 +935,13 @@ CanvasWidgetCmd(clientData, interp, objc, objv)
 	}
 	arg = Tcl_GetStringFromObj(objv[2], (int *) &length);
 	c = arg[0];
+	Tcl_MutexLock(&typeListMutex);
 	for (typePtr = typeList; typePtr != NULL; typePtr = typePtr->nextPtr) {
 	    if ((c == typePtr->name[0])
 		    && (strncmp(arg, typePtr->name, length) == 0)) {
 		if (matchPtr != NULL) {
-		    badType:
+		    Tcl_MutexUnlock(&typeListMutex);
+		  badType:
 		    Tcl_AppendResult(interp,
 			    "unknown or ambiguous item type \"",
 			    arg, "\"", (char *) NULL);
@@ -942,6 +951,12 @@ CanvasWidgetCmd(clientData, interp, objc, objv)
 		matchPtr = typePtr;
 	    }
 	}
+	/*
+	 * Can unlock now because we no longer look at the fields of
+	 * the matched item type that are potentially modified by
+	 * other threads.
+	 */
+	Tcl_MutexUnlock(&typeListMutex);
 	if (matchPtr == NULL) {
 	    goto badType;
 	}
@@ -2700,6 +2715,7 @@ Tk_CreateItemType(typePtr)
      * If there's already an item type with the given name, remove it.
      */
 
+    Tcl_MutexLock(&typeListMutex);
     for (typePtr2 = typeList, prevPtr = NULL; typePtr2 != NULL;
 	    prevPtr = typePtr2, typePtr2 = typePtr2->nextPtr) {
 	if (strcmp(typePtr2->name, typePtr->name) == 0) {
@@ -2713,6 +2729,7 @@ Tk_CreateItemType(typePtr)
     }
     typePtr->nextPtr = typeList;
     typeList = typePtr;
+    Tcl_MutexUnlock(&typeListMutex);
 }
 
 /*
@@ -2721,7 +2738,9 @@ Tk_CreateItemType(typePtr)
  * Tk_GetItemTypes --
  *
  *	This procedure returns a pointer to the list of all item
- *	types.
+ *	types. Note that this is inherently thread-unsafe, but since
+ *	item types are only ever registered very rarely this is
+ *	unlikely to be a problem in practice.
  *
  * Results:
  *	The return value is a pointer to the first in the list
@@ -2748,8 +2767,8 @@ Tk_GetItemTypes()
  * InitCanvas --
  *
  *	This procedure is invoked to perform once-only-ever
- *	initialization for the module, such as setting up
- *	the type table.
+ *	initialization for the module, such as setting up the type
+ *	table.
  *
  * Results:
  *	None.
@@ -2763,7 +2782,9 @@ Tk_GetItemTypes()
 static void
 InitCanvas()
 {
+    Tcl_MutexLock(&typeListMutex);
     if (typeList != NULL) {
+	Tcl_MutexUnlock(&typeListMutex);
 	return;
     }
     typeList = &tkRectangleType;
@@ -2776,18 +2797,7 @@ InitCanvas()
     tkBitmapType.nextPtr = &tkArcType;
     tkArcType.nextPtr = &tkWindowType;
     tkWindowType.nextPtr = NULL;
-#ifndef USE_OLD_TAG_SEARCH
-    allUid = Tk_GetUid("all");
-    currentUid = Tk_GetUid("current");
-    andUid = Tk_GetUid("&&");
-    orUid = Tk_GetUid("||");
-    xorUid = Tk_GetUid("^");
-    parenUid = Tk_GetUid("(");
-    endparenUid = Tk_GetUid(")");
-    negparenUid = Tk_GetUid("!(");
-    tagvalUid = Tk_GetUid("!!");
-    negtagvalUid = Tk_GetUid("!");
-#endif /* USE_OLD_TAG_SEARCH */
+    Tcl_MutexUnlock(&typeListMutex);
 }
 
 #ifdef USE_OLD_TAG_SEARCH
@@ -3000,6 +3010,47 @@ NextItem(searchPtr)
 
 #else /* USE_OLD_TAG_SEARCH */
 /*
+ *----------------------------------------------------------------------
+ *
+ * GetStaticUids --
+ *
+ *This procedure is invoked to return a structure filled with
+ *the Uids used when doing tag searching. If it was never before
+ *called in the current thread, it initializes the structure for
+ *that thread (uids are only ever local to one thread [Bug
+ *1114977]).
+ *
+ * Results:
+ *None.
+ *
+ * Side effects:
+ *None.
+ *
+ *----------------------------------------------------------------------
+ */
+
+static SearchUids *
+GetStaticUids()
+{
+    SearchUids *searchUids = (SearchUids *)
+	    Tcl_GetThreadData(&dataKey, sizeof(SearchUids));
+
+    if (searchUids->allUid == NULL) {
+	searchUids->allUid       = Tk_GetUid("all");
+	searchUids->currentUid   = Tk_GetUid("current");
+	searchUids->andUid       = Tk_GetUid("&&");
+	searchUids->orUid        = Tk_GetUid("||");
+	searchUids->xorUid       = Tk_GetUid("^");
+	searchUids->parenUid     = Tk_GetUid("(");
+	searchUids->endparenUid  = Tk_GetUid(")");
+	searchUids->negparenUid  = Tk_GetUid("!(");
+	searchUids->tagvalUid    = Tk_GetUid("!!");
+	searchUids->negtagvalUid = Tk_GetUid("!");
+    }
+    return searchUids;
+}
+
+/*
  *--------------------------------------------------------------
  *
  * TagSearchExprInit --
@@ -3015,7 +3066,7 @@ NextItem(searchPtr)
 
 static void
 TagSearchExprInit(exprPtrPtr)
-TagSearchExpr **exprPtrPtr;
+    TagSearchExpr **exprPtrPtr;
 {
     TagSearchExpr* expr = *exprPtrPtr;
 
@@ -3200,7 +3251,7 @@ TagSearchScan(canvasPtr, tagObj, searchPtrPtr)
 	}
 	searchPtr->expr->length = searchPtr->expr->index;
     } else {
-        if (searchPtr->expr->uid == allUid) {
+        if (searchPtr->expr->uid == GetStaticUids()->allUid) {
             /*
              * All items match.
              */
@@ -3276,7 +3327,10 @@ TagSearchScanExpr(interp, searchPtr, expr)
     int negate_result;          /* Pending negation of next tag value */
     char *tag;                  /* tag from tag expression string */
     char c;
-
+    SearchUids *searchUids;	/* Collection of uids for basic search
+				 * expression terms. */
+ 
+    searchUids = GetStaticUids();
     negate_result = 0;
     found_tag = 0;
     looking_for_tag = 1;
@@ -3307,8 +3361,8 @@ TagSearchScanExpr(interp, searchPtr, expr)
                 case '!'  :	/* negate next tag or subexpr */
                     if (looking_for_tag > 1) {
                         Tcl_AppendResult(interp,
-                            "Too many '!' in tag search expression",
-                            (char *) NULL);
+				"Too many '!' in tag search expression",
+				(char *) NULL);
                         return TCL_ERROR;
                     }
                     looking_for_tag++;
@@ -3317,10 +3371,10 @@ TagSearchScanExpr(interp, searchPtr, expr)
 
                 case '('  :	/* scan (negated) subexpr recursively */
                     if (negate_result) {
-                        expr->uids[expr->index++] = negparenUid;
+                        expr->uids[expr->index++] = searchUids->negparenUid;
                         negate_result = 0;
 		    } else {
-                        expr->uids[expr->index++] = parenUid;
+                        expr->uids[expr->index++] = searchUids->parenUid;
 		    }
                     if (TagSearchScanExpr(interp, searchPtr, expr) != TCL_OK) {
                         /* Result string should be already set
@@ -3333,10 +3387,10 @@ TagSearchScanExpr(interp, searchPtr, expr)
 
                 case '"'  :	/* quoted tag string */
                     if (negate_result) {
-                        expr->uids[expr->index++] = negtagvalUid;
+                        expr->uids[expr->index++] = searchUids->negtagvalUid;
                         negate_result = 0;
                     } else {
-                        expr->uids[expr->index++] = tagvalUid;
+                        expr->uids[expr->index++] = searchUids->tagvalUid;
 		    }
                     tag = searchPtr->rewritebuffer;
                     found_endquote = 0;
@@ -3365,7 +3419,7 @@ TagSearchScanExpr(interp, searchPtr, expr)
                     }
                     *tag++ = '\0';
                     expr->uids[expr->index++] =
-                        Tk_GetUid(searchPtr->rewritebuffer);
+			    Tk_GetUid(searchPtr->rewritebuffer);
                     looking_for_tag = 0;
                     found_tag = 1;
                     break;
@@ -3381,10 +3435,10 @@ TagSearchScanExpr(interp, searchPtr, expr)
 
                 default :	/* unquoted tag string */
                     if (negate_result) {
-                        expr->uids[expr->index++] = negtagvalUid;
+                        expr->uids[expr->index++] = searchUids->negtagvalUid;
                         negate_result = 0;
                     } else {
-                        expr->uids[expr->index++] = tagvalUid;
+                        expr->uids[expr->index++] = searchUids->tagvalUid;
                     }
                     tag = searchPtr->rewritebuffer;
                     *tag++ = c;
@@ -3409,7 +3463,7 @@ TagSearchScanExpr(interp, searchPtr, expr)
                     }
                     *++tag = '\0';
                     expr->uids[expr->index++] =
-                        Tk_GetUid(searchPtr->rewritebuffer);
+			    Tk_GetUid(searchPtr->rewritebuffer);
                     looking_for_tag = 0;
                     found_tag = 1;
             }
@@ -3431,7 +3485,7 @@ TagSearchScanExpr(interp, searchPtr, expr)
                                 (char *) NULL);
                         return TCL_ERROR;
                     }
-                    expr->uids[expr->index++] = andUid;
+                    expr->uids[expr->index++] = searchUids->andUid;
                     looking_for_tag = 1;
                     break;
 
@@ -3443,17 +3497,17 @@ TagSearchScanExpr(interp, searchPtr, expr)
                                 (char *) NULL);
                         return TCL_ERROR;
                     }
-                    expr->uids[expr->index++] = orUid;
+                    expr->uids[expr->index++] = searchUids->orUid;
                     looking_for_tag = 1;
                     break;
 
                 case '^'  :	/* XOR operator */
-                    expr->uids[expr->index++] = xorUid;
+                    expr->uids[expr->index++] = searchUids->xorUid;
                     looking_for_tag = 1;
                     break;
 
                 case ')'  :	/* end subexpression */
-                    expr->uids[expr->index++] = endparenUid;
+                    expr->uids[expr->index++] = searchUids->endparenUid;
                     goto breakwhile;
 
                 default   :	/* syntax error */
@@ -3503,7 +3557,10 @@ TagSearchEvalExpr(expr, itemPtr)
     int count;
     int result;                 /* Value of expr so far */
     int parendepth;
+    SearchUids *searchUids;	/* Collection of uids for basic search
+				 * expression terms. */
 
+    searchUids = GetStaticUids();
     result = 0;  /* just to keep the compiler quiet */
 
     negate_result = 0;
@@ -3511,7 +3568,7 @@ TagSearchEvalExpr(expr, itemPtr)
     while (expr->index < expr->length) {
         uid = expr->uids[expr->index++];
         if (looking_for_tag) {
-            if (uid == tagvalUid) {
+            if (uid == searchUids->tagvalUid) {
 /*
  *              assert(expr->index < expr->length);
  */
@@ -3528,7 +3585,7 @@ TagSearchEvalExpr(expr, itemPtr)
                     }
                 }
 
-            } else if (uid == negtagvalUid) {
+            } else if (uid == searchUids->negtagvalUid) {
                 negate_result = ! negate_result;
 /*
  *              assert(expr->index < expr->length);
@@ -3546,13 +3603,13 @@ TagSearchEvalExpr(expr, itemPtr)
                     }
                 }
 
-            } else if (uid == parenUid) {
+            } else if (uid == searchUids->parenUid) {
                 /*
                  * evaluate subexpressions with recursion
                  */
                 result = TagSearchEvalExpr(expr, itemPtr);
 
-            } else if (uid == negparenUid) {
+            } else if (uid == searchUids->negparenUid) {
                 negate_result = ! negate_result;
                 /*
                  * evaluate subexpressions with recursion
@@ -3569,7 +3626,8 @@ TagSearchEvalExpr(expr, itemPtr)
             }
             looking_for_tag = 0;
         } else {    /* ! looking_for_tag */
-            if (((uid == andUid) && (!result)) || ((uid == orUid) && result)) {
+            if (((uid == searchUids->andUid) && (!result)) ||
+		    ((uid == searchUids->orUid) && result)) {
                 /*
                  * short circuit expression evaluation
                  *
@@ -3581,31 +3639,33 @@ TagSearchEvalExpr(expr, itemPtr)
                     parendepth = 0;
 		while (expr->index < expr->length) {
 		    uid = expr->uids[expr->index++];
-		    if (uid == tagvalUid || uid == negtagvalUid) {
+		    if (uid == searchUids->tagvalUid ||
+			    uid == searchUids->negtagvalUid) {
 			expr->index++;
 			continue;
 		    }
-                        if (uid == parenUid || uid == negparenUid) {
-                            parendepth++;
+		    if (uid == searchUids->parenUid ||
+			    uid == searchUids->negparenUid) {
+			parendepth++;
 			continue;
 		    } 
-		    if (uid == endparenUid) {
-                            parendepth--;
-                            if (parendepth < 0) {
-                                break;
-                            }
-                        }
-                    }
+		    if (uid == searchUids->endparenUid) {
+			parendepth--;
+			if (parendepth < 0) {
+			    break;
+			}
+		    }
+		}
                 return result;
 
-            } else if (uid == xorUid) {
+            } else if (uid == searchUids->xorUid) {
                 /*
                  * if the previous result was 1
                  *   then negate the next result
                  */
                 negate_result = result;
 
-            } else if (uid == endparenUid) {
+            } else if (uid == searchUids->endparenUid) {
                 return result;
 /*
  *          } else {
@@ -3669,10 +3729,10 @@ TagSearchFirst(searchPtr)
 
         itemPtr = searchPtr->canvasPtr->hotPtr;
         lastPtr = searchPtr->canvasPtr->hotPrevPtr;
-        if ((itemPtr == NULL) || (itemPtr->id != searchPtr->id) || (lastPtr == NULL)
-            || (lastPtr->nextPtr != itemPtr)) {
+        if ((itemPtr == NULL) || (itemPtr->id != searchPtr->id) ||
+		(lastPtr == NULL) || (lastPtr->nextPtr != itemPtr)) {
             entryPtr = Tcl_FindHashEntry(&searchPtr->canvasPtr->idTable,
-                (char *) searchPtr->id);
+		    (char *) searchPtr->id);
             if (entryPtr != NULL) {
                 itemPtr = (Tk_Item *)Tcl_GetHashValue(entryPtr);
                 lastPtr = itemPtr->prevPtr;
@@ -3718,19 +3778,19 @@ TagSearchFirst(searchPtr)
         }
     } else {
 
-    /*
-         * None of the above.  Search for an item matching the tag expression.
-     */
+	/*
+	 * None of the above.  Search for an item matching the tag expression.
+	 */
 
-    for (lastPtr = NULL, itemPtr = searchPtr->canvasPtr->firstItemPtr;
-                itemPtr != NULL; lastPtr = itemPtr, itemPtr = itemPtr->nextPtr) {
+	for (lastPtr = NULL, itemPtr = searchPtr->canvasPtr->firstItemPtr;
+		itemPtr != NULL; lastPtr=itemPtr, itemPtr=itemPtr->nextPtr) {
 	    searchPtr->expr->index = 0;
 	    if (TagSearchEvalExpr(searchPtr->expr, itemPtr)) {
-            searchPtr->lastPtr = lastPtr;
-            searchPtr->currentPtr = itemPtr;
-            return itemPtr;
-        }
-        }
+		searchPtr->lastPtr = lastPtr;
+		searchPtr->currentPtr = itemPtr;
+		return itemPtr;
+	    }
+	}
     }
     searchPtr->lastPtr = lastPtr;
     searchPtr->searchOver = 1;
@@ -4574,6 +4634,9 @@ PickCurrentItem(canvasPtr, eventPtr)
     double coords[2];
     int buttonDown;
     Tk_Item *prevItemPtr;
+#ifndef USE_OLD_TAG_SEARCH
+    SearchUids *searchUids = GetStaticUids();
+#endif
 
     /*
      * Check whether or not a button is down.  If so, we'll log entry
@@ -4698,7 +4761,7 @@ PickCurrentItem(canvasPtr, eventPtr)
 #ifdef USE_OLD_TAG_SEARCH
 		if (itemPtr->tagPtr[i] == Tk_GetUid("current")) {
 #else /* USE_OLD_TAG_SEARCH */
-		if (itemPtr->tagPtr[i] == currentUid) {
+		if (itemPtr->tagPtr[i] == searchUids->currentUid) {
 #endif /* USE_OLD_TAG_SEARCH */
 		    itemPtr->tagPtr[i] = itemPtr->tagPtr[itemPtr->numTags-1];
 		    itemPtr->numTags--;
@@ -4741,7 +4804,8 @@ PickCurrentItem(canvasPtr, eventPtr)
 	DoItem((Tcl_Interp *) NULL, canvasPtr->currentItemPtr, 
                 Tk_GetUid("current"));
 #else /* USE_OLD_TAG_SEARCH */
-	DoItem((Tcl_Interp *) NULL, canvasPtr->currentItemPtr, currentUid);
+	DoItem((Tcl_Interp *) NULL, canvasPtr->currentItemPtr,
+		searchUids->currentUid);
 #endif /* USE_OLD_TAG_SEA */
 	if ((canvasPtr->currentItemPtr->redraw_flags & TK_ITEM_STATE_DEPENDANT &&
 		prevItemPtr != canvasPtr->currentItemPtr)) {
@@ -4847,6 +4911,7 @@ CanvasDoEvent(canvasPtr, eventPtr)
 #ifndef USE_OLD_TAG_SEARCH
     TagSearchExpr *expr;
     int numExprs;
+    SearchUids *searchUids = GetStaticUids();
 #endif /* not USE_OLD_TAG_SEARCH */
 
     if (canvasPtr->bindingTable == NULL) {
@@ -4903,13 +4968,13 @@ CanvasDoEvent(canvasPtr, eventPtr)
     if (numObjects <= NUM_STATIC) {
 	objectPtr = staticObjects;
     } else {
-	objectPtr = (ClientData *) ckalloc((unsigned)
-		(numObjects * sizeof(ClientData)));
+	objectPtr = (ClientData *)
+		ckalloc((unsigned) (numObjects * sizeof(ClientData)));
     }
 #ifdef USE_OLD_TAG_SEARCH
     objectPtr[0] = (ClientData) Tk_GetUid("all");
 #else /* USE_OLD_TAG_SEARCH */
-    objectPtr[0] = (ClientData) allUid;
+    objectPtr[0] = (ClientData) searchUids->allUid;
 #endif /* USE_OLD_TAG_SEARCH */
     for (i = itemPtr->numTags-1; i >= 0; i--) {
 	objectPtr[i+1] = (ClientData) itemPtr->tagPtr[i];