From db7a74fbb9341b75d08e2e34917be5d6fb94598c Mon Sep 17 00:00:00 2001 From: dkf Date: Fri, 3 Jun 2005 23:22:39 +0000 Subject: Backport fix for [Bug 1114977]; canvas tag searches now work with threads. --- ChangeLog | 5 ++ generic/tkCanvas.c | 227 ++++++++++++++++++++++++++++++++++------------------- 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 + + * generic/tkCanvas.c (SearchUids): Move all global data into + protected storage to stop cross-thread issues. [Bug 1114977] + 2005-06-03 Donal K. Fellows * 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]; -- cgit v0.12