From e2e552dd43b87233b672688f9577618b1ce468db Mon Sep 17 00:00:00 2001
From: dkf <donal.k.fellows@manchester.ac.uk>
Date: Thu, 3 Feb 2005 13:44:58 +0000
Subject: Ensure that the canvas's static data is either protected by a mutex
or in a thread-local variable. [Bug 1114977]
---
ChangeLog | 7 +++
generic/tkCanvas.c | 177 ++++++++++++++++++++++++++++++++++++-----------------
2 files changed, 128 insertions(+), 56 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index bded451..f375c16 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@
+2005-02-03 Donal K. Fellows <donal.k.fellows@manchester.ac.uk>
+
+ * generic/tkCanvas.c (GetStaticUids): New function to manage the
+ thread-specific data detailing the list of all uids in a thread.
+ (typeList): Protect this (the other piece of global data) with a
+ mutex. [Bug 1114977]
+
2005-01-31 Jeff Hobbs <jeffh@ActiveState.com>
* unix/tcl.m4, unix/configure: add solaris-64 gcc build
diff --git a/generic/tkCanvas.c b/generic/tkCanvas.c
index 839c739..c57d4c6 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.31 2004/06/15 21:37:00 dkf Exp $
+ * RCS: @(#) $Id: tkCanvas.c,v 1.32 2005/02/03 13:44:58 dkf Exp $
*/
/* #define USE_OLD_TAG_SEARCH 1 */
@@ -216,27 +216,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 */
/*
@@ -946,11 +953,13 @@ CanvasWidgetCmd(clientData, interp, objc, objv)
}
arg = Tcl_GetStringFromObj(objv[2], (int *) &length);
c = arg[0];
+ Tcl_MutexLock(&typeListPtr);
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(&typeListPtr);
+ badType:
Tcl_AppendResult(interp,
"unknown or ambiguous item type \"",
arg, "\"", (char *) NULL);
@@ -960,6 +969,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(&typeListPtr);
if (matchPtr == NULL) {
goto badType;
}
@@ -2584,6 +2599,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) {
@@ -2597,6 +2613,7 @@ Tk_CreateItemType(typePtr)
}
typePtr->nextPtr = typeList;
typeList = typePtr;
+ Tcl_MutexUnlock(&typeListMutex);
}
/*
@@ -2605,7 +2622,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
@@ -2627,13 +2646,13 @@ 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.
@@ -2641,13 +2660,15 @@ Tk_GetItemTypes()
* Side effects:
* None.
*
- *--------------------------------------------------------------
+ *----------------------------------------------------------------------
*/
static void
InitCanvas()
{
+ Tcl_MutexLock(&typeListMutex);
if (typeList != NULL) {
+ Tcl_MutexUnlock(&typeListMutex);
return;
}
typeList = &tkRectangleType;
@@ -2660,18 +2681,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
@@ -2882,7 +2892,48 @@ NextItem(searchPtr)
return NULL;
}
-#else /* USE_OLD_TAG_SEARCH */
+#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;
+}
+
/*
*--------------------------------------------------------------
*
@@ -3081,7 +3132,7 @@ TagSearchScan(canvasPtr, tagObj, searchPtrPtr)
return TCL_ERROR;
}
searchPtr->expr->length = searchPtr->expr->index;
- } else if (searchPtr->expr->uid == allUid) {
+ } else if (searchPtr->expr->uid == GetStaticUids()->allUid) {
/*
* All items match.
*/
@@ -3157,7 +3208,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;
@@ -3198,10 +3252,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) {
/*
@@ -3216,10 +3270,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;
@@ -3264,10 +3318,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;
@@ -3313,7 +3367,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;
@@ -3325,17 +3379,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 */
@@ -3385,7 +3439,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;
@@ -3393,7 +3450,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);
*/
@@ -3410,7 +3467,7 @@ TagSearchEvalExpr(expr, itemPtr)
}
}
- } else if (uid == negtagvalUid) {
+ } else if (uid == searchUids->negtagvalUid) {
negate_result = ! negate_result;
/*
* assert(expr->index < expr->length);
@@ -3428,13 +3485,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
@@ -3451,7 +3508,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
*
@@ -3463,15 +3521,17 @@ 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) {
+ if (uid == searchUids->parenUid ||
+ uid == searchUids->negparenUid) {
parendepth++;
continue;
}
- if (uid == endparenUid) {
+ if (uid == searchUids->endparenUid) {
parendepth--;
if (parendepth < 0) {
break;
@@ -3480,14 +3540,14 @@ TagSearchEvalExpr(expr, itemPtr)
}
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 {
@@ -4413,6 +4473,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
@@ -4537,7 +4600,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 */
/* then */ {
itemPtr->tagPtr[i] = itemPtr->tagPtr[itemPtr->numTags-1];
@@ -4581,7 +4644,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)) {
@@ -4687,6 +4751,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) {
@@ -4749,7 +4814,7 @@ CanvasDoEvent(canvasPtr, eventPtr)
#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