Fix potential segfaults when a display update is requested in the middle of drawing items.

5 files changed, 120 insertions, 19 deletions

diff --git a/generic/tkTreeCtrl.h b/generic/tkTreeCtrl.h
index 21ff1de..a021e15 100644
--- a/generic/tkTreeCtrl.h
+++ b/generic/tkTreeCtrl.h
@@ -7,7 +7,7 @@
  * Copyright (c) 2002-2003 Christian Krone
  * Copyright (c) 2003 ActiveState Corporation
  *
- * RCS: @(#) $Id: tkTreeCtrl.h,v 1.74 2006/11/21 01:55:13 treectrl Exp $
+ * RCS: @(#) $Id: tkTreeCtrl.h,v 1.75 2006/11/25 20:25:28 treectrl Exp $
  */
 
 #include "tkPort.h"
@@ -764,6 +764,8 @@ extern void Tree_FreeItemDInfo(TreeCtrl *tree, TreeItem item1, TreeItem item2);
 extern void Tree_InvalidateItemDInfo(TreeCtrl *tree, TreeColumn column, TreeItem item1, TreeItem item2);
 extern void TreeDisplay_ItemDeleted(TreeCtrl *tree, TreeItem item);
 extern void TreeDisplay_ColumnDeleted(TreeCtrl *tree, TreeColumn column);
+extern void TreeDisplay_GetReadyForTrouble(TreeCtrl *tree, int *requestsPtr);
+extern int TreeDisplay_WasThereTrouble(TreeCtrl *tree, int requests);
 extern void Tree_InvalidateArea(TreeCtrl *tree, int x1, int y1, int x2, int y2);
 extern void Tree_InvalidateItemArea(TreeCtrl *tree, int x1, int y1, int x2, int y2);
 extern void Tree_InvalidateRegion(TreeCtrl *tree, TkRegion region);
diff --git a/generic/tkTreeDisplay.c b/generic/tkTreeDisplay.c
index 7dada88..b6993ed 100644
--- a/generic/tkTreeDisplay.c
+++ b/generic/tkTreeDisplay.c
@@ -5,7 +5,7 @@
  *
  * Copyright (c) 2002-2006 Tim Baker
  *
- * RCS: @(#) $Id: tkTreeDisplay.c,v 1.64 2006/11/19 23:37:25 treectrl Exp $
+ * RCS: @(#) $Id: tkTreeDisplay.c,v 1.65 2006/11/25 20:25:28 treectrl Exp $
  */
 
 #include "tkTreeCtrl.h"
@@ -2461,8 +2461,10 @@ DItem_Free(
     if (strncmp(dItem->magic, "MAGC", 4) != 0)
 	panic("DItem_Free: dItem.magic != MAGC");
 #endif
-    if (dItem->item != NULL)
+    if (dItem->item != NULL) {
 	TreeItem_SetDInfo(tree, dItem->item, (TreeItemDInfo) NULL);
+	dItem->item = NULL;
+    }
     /* Push unused DItem on the stack */
     dItem->next = dInfo->dItemFree;
     dInfo->dItemFree = dItem;
@@ -5148,6 +5150,53 @@ DebugDrawBorder(
 /*
  *--------------------------------------------------------------
  *
+ * TreeDisplay_GetReadyForTrouble --
+ * TreeDisplay_WasThereTrouble --
+ *
+ *	These 2 procedures are used to detect when something happens
+ *	during a display update that requests another display update.
+ *	If that happens, then the current display is aborted and we
+ *	try again (unless the window was destroyed).
+ *
+ * Results:
+ *	None.
+ *
+ * Side effects:
+ *	None.
+ *
+ *--------------------------------------------------------------
+ */
+
+void
+TreeDisplay_GetReadyForTrouble(
+    TreeCtrl *tree,
+    int *requestsPtr
+    )
+{
+    TreeDInfo dInfo = tree->dInfo;
+
+    *requestsPtr = dInfo->requests;
+}
+
+int
+TreeDisplay_WasThereTrouble(
+    TreeCtrl *tree,
+    int requests
+    )
+{
+    TreeDInfo dInfo = tree->dInfo;
+
+    if (tree->deleted || (requests != dInfo->requests)) {
+	if (tree->debug.enable)
+	    dbwin("TreeDisplay_WasThereTrouble: %p\n", tree);
+	return 1;
+    }
+    return 0;
+}
+
+/*
+ *--------------------------------------------------------------
+ *
  * Tree_Display --
  *
  *	This procedure is called at idle time when something has happened
@@ -5332,7 +5381,7 @@ displayRetry:
      * If binding scripts do something that causes a redraw to be requested,
      * then we abort the current draw and start again.
      */
-    requests = dInfo->requests;
+    TreeDisplay_GetReadyForTrouble(tree, &requests);
     if (dInfo->flags & DINFO_UPDATE_SCROLLBAR_X) {
 	/* Possible <Scroll-x> event. */
 	Tree_UpdateScrollbarX(tree);
@@ -5345,7 +5394,7 @@ displayRetry:
     }
     if (tree->deleted || !Tk_IsMapped(tkwin))
 	goto displayExit;
-    if (requests != dInfo->requests) {
+    if (TreeDisplay_WasThereTrouble(tree, requests)) {
 	goto displayRetry;
     }
     if (dInfo->flags & DINFO_OUT_OF_DATE) {
@@ -5421,8 +5470,6 @@ displayRetry:
 #endif /* DCOLUMN */
 	}
 
-	requests = dInfo->requests;
-
 	/*
 	 * Generate an <ItemVisibility> event here. This can be used to set
 	 * an item's styles when the item is about to be displayed, and to
@@ -5438,9 +5485,8 @@ displayRetry:
 	if (tree->deleted || !Tk_IsMapped(tkwin))
 	    goto displayExit;
 
-	if (requests != dInfo->requests) {
+	if (TreeDisplay_WasThereTrouble(tree, requests))
 	    goto displayRetry;
-	}
     }
 
     if (tree->doubleBuffer == DOUBLEBUFFER_WINDOW) {
@@ -5636,11 +5682,15 @@ displayRetry:
 
 	    int drawn = 0;
 	    if (!dInfo->empty && dInfo->rangeFirst != NULL) {
-		/* FIXME: may "draw" window elements twice. */
 		tree->drawableXOrigin = tree->xOrigin;
 		tree->drawableYOrigin = tree->yOrigin;
 		TreeItem_UpdateWindowPositions(tree, dItem->item, COLUMN_LOCK_NONE,
 		    dItem->area.x, dItem->y, dItem->area.width, dItem->height);
+		if (TreeDisplay_WasThereTrouble(tree, requests)) {
+		    if (tree->deleted || !Tk_IsMapped(tree->tkwin))
+			goto displayExit;
+		    goto displayRetry;
+		}
 		if (dItem->area.flags & DITEM_DIRTY) {
 		    drawn += DisplayDItem(tree, dItem, &dItem->area,
 			    COLUMN_LOCK_NONE, dInfo->bounds, pixmap, drawable);
@@ -5652,6 +5702,11 @@ displayRetry:
 		TreeItem_UpdateWindowPositions(tree, dItem->item,
 		    COLUMN_LOCK_LEFT, dItem->left.x, dItem->y,
 		    dItem->left.width, dItem->height);
+		if (TreeDisplay_WasThereTrouble(tree, requests)) {
+		    if (tree->deleted || !Tk_IsMapped(tree->tkwin))
+			goto displayExit;
+		    goto displayRetry;
+		}
 		if (dItem->left.flags & DITEM_DIRTY) {
 		    drawn += DisplayDItem(tree, dItem, &dItem->left, COLUMN_LOCK_LEFT,
 			    dInfo->boundsL, pixmap, drawable);
@@ -5663,6 +5718,11 @@ displayRetry:
 		TreeItem_UpdateWindowPositions(tree, dItem->item,
 		    COLUMN_LOCK_RIGHT, dItem->right.x, dItem->y,
 		    dItem->right.width, dItem->height);
+		if (TreeDisplay_WasThereTrouble(tree, requests)) {
+		    if (tree->deleted || !Tk_IsMapped(tree->tkwin))
+			goto displayExit;
+		    goto displayRetry;
+		}
 		if (dItem->right.flags & DITEM_DIRTY) {
 		    drawn += DisplayDItem(tree, dItem, &dItem->right, COLUMN_LOCK_RIGHT,
 			    dInfo->boundsR, pixmap, drawable);
@@ -5670,7 +5730,7 @@ displayRetry:
 	    }
 	    numDraw += drawn ? 1 : 0;
 
-	    dItem->oldX = dItem->area.x;
+	    dItem->oldX = dItem->area.x; /* FIXME: could have dInfo->empty */
 	    dItem->oldY = dItem->y;
 	    dItem->oldIndex = dItem->index;
 	}
diff --git a/generic/tkTreeElem.c b/generic/tkTreeElem.c
index 70a6dc8..f82293c 100644
--- a/generic/tkTreeElem.c
+++ b/generic/tkTreeElem.c
@@ -5,7 +5,7 @@
  *
  * Copyright (c) 2002-2006 Tim Baker
  *
- * RCS: @(#) $Id: tkTreeElem.c,v 1.51 2006/11/23 00:42:26 treectrl Exp $
+ * RCS: @(#) $Id: tkTreeElem.c,v 1.52 2006/11/25 20:25:28 treectrl Exp $
  */
 
 #include "tkTreeCtrl.h"
@@ -3720,6 +3720,7 @@ static void DisplayProcWindow(ElementArgs *args)
     int width, height;
     int match, match2;
     int draw;
+    int requests;
 
     BOOLEAN_FOR_STATE(draw, draw, state);
     if (!draw)
@@ -3778,6 +3779,8 @@ hideIt:
 	return;
     }
 
+    TreeDisplay_GetReadyForTrouble(tree, &requests);
+
 #ifdef CLIP_WINDOW
     if (elemX->child != NULL) {
 	int cx = x, cy = y, cw = width, ch = height; /* clip win coords */
@@ -3805,11 +3808,15 @@ hideIt:
 		    || (cw != Tk_Width(elemX->tkwin))
 		    || (ch != Tk_Height(elemX->tkwin))) {
 		Tk_MoveResizeWindow(elemX->tkwin, cx, cy, cw, ch);
+		if (TreeDisplay_WasThereTrouble(tree, requests))
+		    return;
 	    }
 	    Tk_MapWindow(elemX->tkwin);
 	} else {
 	    Tk_MaintainGeometry(elemX->tkwin, tree->tkwin, cx, cy, cw, ch);
 	}
+	if (TreeDisplay_WasThereTrouble(tree, requests))
+	    return;
 
 	/*
 	 * Position the child window within the clip window.
@@ -3820,6 +3827,8 @@ hideIt:
 		|| (width != Tk_Width(elemX->child))
 		|| (height != Tk_Height(elemX->child))) {
 	    Tk_MoveResizeWindow(elemX->child, x, y, width, height);
+	    if (TreeDisplay_WasThereTrouble(tree, requests))
+		return;
 	}
 	Tk_MapWindow(elemX->child);
 	return;
@@ -3836,6 +3845,8 @@ hideIt:
 		|| (width != Tk_Width(elemX->tkwin))
 		|| (height != Tk_Height(elemX->tkwin))) {
 	    Tk_MoveResizeWindow(elemX->tkwin, x, y, width, height);
+	    if (TreeDisplay_WasThereTrouble(tree, requests))
+		return;
 	}
 	Tk_MapWindow(elemX->tkwin);
     } else {
diff --git a/generic/tkTreeItem.c b/generic/tkTreeItem.c
index f7753d0..87debaf 100644
--- a/generic/tkTreeItem.c
+++ b/generic/tkTreeItem.c
@@ -5,7 +5,7 @@
  *
  * Copyright (c) 2002-2006 Tim Baker
  *
- * RCS: @(#) $Id: tkTreeItem.c,v 1.93 2006/11/22 03:32:12 treectrl Exp $
+ * RCS: @(#) $Id: tkTreeItem.c,v 1.94 2006/11/25 20:25:28 treectrl Exp $
  */
 
 #include "tkTreeCtrl.h"
@@ -4404,15 +4404,21 @@ SpanWalkProc_UpdateWindowPositions(
     )
 {
     StyleDrawArgs drawArgsCopy;
+    int requests;
 
     if ((drawArgs->x >= drawArgs->bounds[2]) ||
 	    (drawArgs->x + drawArgs->width <= drawArgs->bounds[0]) ||
 	    (drawArgs->style == NULL))
 	return 0;
 
+    TreeDisplay_GetReadyForTrouble(tree, &requests);
+
     drawArgsCopy = *drawArgs; 
     TreeStyle_UpdateWindowPositions(&drawArgsCopy);
 
+    if (TreeDisplay_WasThereTrouble(tree, requests))
+	return 1;
+
     /* Stop walking if we went past the right edge of the display area. */
     return drawArgs->x + drawArgs->width >= drawArgs->bounds[2];
 }
diff --git a/generic/tkTreeStyle.c b/generic/tkTreeStyle.c
index ef2a19f..8d9e21f 100644
--- a/generic/tkTreeStyle.c
+++ b/generic/tkTreeStyle.c
@@ -5,7 +5,7 @@
  *
  * Copyright (c) 2002-2006 Tim Baker
  *
- * RCS: @(#) $Id: tkTreeStyle.c,v 1.64 2006/11/23 22:04:22 treectrl Exp $
+ * RCS: @(#) $Id: tkTreeStyle.c,v 1.65 2006/11/25 20:25:28 treectrl Exp $
  */
 
 #include "tkTreeCtrl.h"
@@ -2299,6 +2299,11 @@ void TreeStyle_Draw(
     {
 	struct Layout *layout = &layouts[i];
 
+	/* Don't "draw" window elements. TreeStyle_UpdateWindowPositions()
+	 * does that for us. */
+	if (ELEMENT_TYPE_MATCHES(layout->eLink->elem->typePtr, &elemTypeWindow))
+	    continue;
+
 #if DEBUG_DRAW
 	if (debugDraw && layout->master->onion != NULL)
 	    continue;
@@ -2435,14 +2440,15 @@ TreeStyle_UpdateWindowPositions(
     ElementArgs args;
     int i, x, y, minWidth, minHeight;
     struct Layout staticLayouts[STATIC_SIZE], *layouts = staticLayouts;
+    int numElements = masterStyle->numElements;
 
     /* FIXME: Perhaps remember whether this style has any window
      * elements */
-    for (i = 0; i < masterStyle->numElements; i++) {
+    for (i = 0; i < numElements; i++) {
 	if (ELEMENT_TYPE_MATCHES(masterStyle->elements[i].elem->typePtr, &elemTypeWindow))
 	    break;
     }
-    if (i == masterStyle->numElements)
+    if (i == numElements)
 	return;
 
     Style_CheckNeededSize(tree, style, drawArgs->state);
@@ -2474,7 +2480,7 @@ TreeStyle_UpdateWindowPositions(
     if (drawArgs->height < minHeight)
 	drawArgs->height = minHeight;
 
-    STATIC_ALLOC(layouts, struct Layout, style->master->numElements);
+    STATIC_ALLOC(layouts, struct Layout, numElements);
 
     Style_DoLayout(drawArgs, layouts, FALSE, __FILE__, __LINE__);
 
@@ -2482,7 +2488,7 @@ TreeStyle_UpdateWindowPositions(
     args.state = drawArgs->state;
     args.display.drawable = drawArgs->drawable;
 
-    for (i = 0; i < style->master->numElements; i++)
+    for (i = 0; i < numElements; i++)
     {
 	struct Layout *layout = &layouts[i];
 
@@ -2491,6 +2497,10 @@ TreeStyle_UpdateWindowPositions(
 
 	if ((layout->useWidth > 0) && (layout->useHeight > 0))
 	{
+	    int requests;
+
+	    TreeDisplay_GetReadyForTrouble(tree, &requests);
+
 	    args.elem = layout->eLink->elem;
 	    args.display.x = drawArgs->x + layout->x + layout->ePadX[PAD_TOP_LEFT];
 	    args.display.y = drawArgs->y + layout->y + layout->ePadY[PAD_TOP_LEFT];
@@ -2500,10 +2510,22 @@ TreeStyle_UpdateWindowPositions(
 	    args.display.height = layout->useHeight;
 	    args.display.sticky = layout->master->flags & ELF_STICKY;
 	    (*args.elem->typePtr->displayProc)(&args);
+
+	    /* Updating the position of a window may generate a <Configure>
+	     * or <Map> event on that window. Binding scripts on those
+	     * events could do anything, including deleting items and
+	     * thus the style we are drawing. In other cases (such as when
+	     * using Tile widgets I notice), the Tk_GeomMgr.requestProc
+	     * may get called which calls Tree_ElementChangedItself which
+	     * calls FreeDItemInfo which frees a DItem we are in the middle
+	     * of displaying. So if anything was done that caused a display
+	     * request, then abort abort abort. */
+	    if (TreeDisplay_WasThereTrouble(tree, requests))
+		break;
 	}
     }
 
-    STATIC_FREE(layouts, struct Layout, style->master->numElements);
+    STATIC_FREE(layouts, struct Layout, numElements);
 }
 
 /*