alan@metasoft.UUCP (Alan Epstein) (10/05/90)
Recently our application began crashing reliably during a mouse down event in a small widget hierarchy. We traced the problem to _XtFindRemapWidget, which makes a preliminary test to determine whether the pdi->trace array should be refilled (by calling _XtFillAncestorList). In some cases where it decides not to refill the array, the existing array contains garbage, causing a crash later when _XtGetPerWidgetInput tries to dereference through the invalid widget. The bug appears to be due to the possibility that Xt reassigns widget id's, ie: after a widget is destroyed (and presumably its id is freed), XtCreate might return the same id. This new id is valid, but the result is that the test in _XtFindRemapWidget might succeed (ie: widget == pdi->trace[0]) when the other elements in the pdi->trace array are old and obsolete. The fix I am trying involves merely removing the test at the head of _XtFindRemapWidget, causing _XtFillAncestorList to make the array right every time. This seems like little overhead since the latter routine doesn't need to do much. I don't profess to be an expert in the Xt code, so I would appreciate a confirmation by someone who is. ----------------------------- Alan Epstein Meta Software Corp UUCP: ...bbn!metasoft!alan 150 Cambridgepark Dr Internet/ARPA: alan%metasoft@bbn.com Cambridge, MA 02140 USA TEL: (617) 576.6920 -----------------------------
alan@metasoft.UUCP (Alan Epstein) (10/08/90)
In article <1874@metasoft.UUCP>, alan@metasoft.UUCP (Alan Epstein) writes: > Recently our application began crashing reliably during a mouse down... I just looked at patches 15-17 and noticed this had already been fixed. Never Mind!.... Please send replies via e-mail. Avoid net clutter! Thanks. ----------------------------- Alan Epstein Meta Software Corp UUCP: ...bbn!metasoft!alan 150 Cambridgepark Dr Internet/ARPA: alan%metasoft@bbn.com Cambridge, MA 02140 USA Cambridge, MA 02140 USA IAP: ILS 11 BED Cambridge, MA 02140 USA TEL: (617) 576.6920 -----------------------------
gourley@CAD.MCC.COM (Kevin Gourley) (10/08/90)
Alan, you ran into the same bug that I had experienced over a month
ago. I sent out a message to xpert about this bug and I later
sent it in as a bug report with suggested patch to xbug. I have
included my original message below. After a month of testing,
my patch seems to have fixed the problem!
We recently applied the latest MIT patches. After looking into
the code, it appears to me that they also have fixed the problem.
You are right. You probably DON'T want to call _XtFillAncestorList
every time. You probablly DO want to apply the latest patches though.
Kevin Gourley (gourley@mcc.com) 512-338-3589
Microelectronics and Computer Technology Corp., Austin, TX
My Original message about this bug is below
vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
I have been experiencing a problem that has been causing 'bus error's
frequently since we started using GNU malloc. I finally tracked it down
to a bug in Xt. The use of GNU malloc only makes the bug more apparent.
The problem is in EventUtil.c in _XtFindRemapWidget:
Widget _XtFindRemapWidget(event, widget, mask, pdi)
XEvent *event;
Widget widget;
EventMask mask;
XtPerDisplayInput pdi;
{
Widget dspWidget = widget;
+-> if (!pdi->traceDepth || !(widget == pdi->trace[0]))
| {
| _XtFillAncestorList(&pdi->trace, &pdi->traceMax,
| &pdi->traceDepth, widget, NULL);
| pdi->focusWidget = NULL; /* invalidate the focus
| cache */
| }
| ...
|
|
This 'if' condition leads to erroneous results. pdi retains
a cached snapshot of the path up the widget hierarchy in
pdi->trace (which is an array of widget pointers). These values are
recomputed by calling _XtFillAncestorList whenever it is
determined that the cache is no longer valid. The condition in
the 'if' statement is NOT SUFFICIENT to adequately check to see
if the pdi->trace array is still valid.
I had some code in a rather complex application that created a dialog
box composed of a hierarchy of widgets. Upon interacting with that
dialog, values would get cached in the pdi->trace array. Then at some
later point, I would end up destroying the widgets and their memory
would get 'free'd. Later, when I created a new dialog composed of
freshly allocated widgets, I would occasionally encounter cases where
some of the new widgets coincided in memory location with the
addresses of widgets in the previous dialog. (This was a result of a
smarter memory allocator). The problem is that even though I have a
new hierarchy of widgets, the '(widget == pdi->trace[0])' test was
erroneous because sometimes the widget pointer WAS indeed the same as
the cached widget pointer value in pdi->trace[0] BUT it was not the
SAME widget. Since the time that the widget pointer value was cached,
that widget was freed and a new widget was allocated in the same
space. That was the only thing that was the same though! The rest of
the pdi->trace array did not reflect the current path up the widget
hierarchy and in fact contained stale pointers to widgets that had
been freed. Since the 'if' clause did not detect the erroneous
pdi->trace array, it did not recompute the values by calling
_XtFillAncestorList and it resulted in 'bus errors' further down in
the code.
This doesn't appear to be as much of a problem in other forms of
malloc because new widgets don't tend to get allocated in the exact
same location as older freed widgets.
My solution is the following:
1) I created a function in EventUtil.c that resets the pdi->traceDepth
to 0, thus causing the pdi->trace array to be recomputed when necessary.
void _XtClearPerDisplayInputCache(dpy)
Display *dpy;
{
XtPerDisplayInput pdi;
pdi = _XtGetPerDisplayInput(dpy);
if (pdi) pdi->traceDepth = 0;
}
2) I changed a function in Destroy.c that now calls
_XtClearPerDisplayInputCache whenever a widget is desroyed to mark
the cache as "dirty".
static void Phase1Destroy (widget)
Widget widget;
{
> extern void _XtClearPerDisplayInputCache();
> Screen *screen;
widget->core.being_destroyed = TRUE;
> if (screen = XtScreenOfObject(widget))
> _XtClearPerDisplayInputCache(DisplayOfScreen(screen));
} /* Phase1Destroy */
Please let me know if this has been fixed by a patch or if my solution is
not correct. Thank you.
/ ____
/__/ ___ o ___ / ___ / ___
/\ /__/ | / / / / / --- / / / / /-- / /__/ | /
/ | /__ |/ / / / /___/ /__/ /__/ / / /__ |/
___________________________________________________/
Microelectronics & Computer Tech.Corp.(MCC), CAD Dept.
3500 W.Balcones Center Dr., Austin,TX 78759
gourley@mcc.com (512)338-3589 FAX:(512)338-3600
----------------------------------
In 'diff' form, the patches are:
Xt/EventUtil.c:
74a75,84
> void _XtClearPerDisplayInputCache(dpy)
> Display *dpy;
> {
> XtPerDisplayInput pdi;
>
> pdi = _XtGetPerDisplayInput(dpy);
> if (pdi) pdi->traceDepth = 0;
> }
>
>
Xt/Destroy.c:
59a60,62
> extern void _XtClearPerDisplayInputCache();
> Screen *screen;
>
60a64,67
>
> if (screen = XtScreen(widget))
> _XtClearPerDisplayInputCache(DisplayOfScreen(screen));
>
----------------------------------