#559 NEdit crashed on window close

[Thorsten Haude]

From a thread starting here:
http://www.nedit.org/pipermail/develop/2006-October/012649.html

>I just got a freezing NEdit:
>#0 0x08067af1 in WidgetToWindow (w=0x852c0d8) at
window.c:2037
>#1 0x0806bc91 in GetTopDocument (w=0x89) at window.c:4112
>#2 0x0806bcb7 in IsTopDocument (window=0x8514f28) at
window.c:4119
>#3 0x08067c34 in UpdateWindowTitle (window=0x8514f28)
at window.c:2075
>#4 0x0805608c in CheckForChangesToFile
(window=0x8514f28) at file.c:1968
>#5 0x418ee699 in XtCallCallbacks () from
/usr/X11R6/lib/libXt.so.6
>#6 0x419226c7 in _XtMatchAtom () from
/usr/X11R6/lib/libXt.so.6
>#7 0x41922bfb in _XtMatchAtom () from
/usr/X11R6/lib/libXt.so.6
>#8 0x419231f3 in _XtTranslateEvent () from
/usr/X11R6/lib/libXt.so.6
>#9 0x418fb73b in XtDispatchEventToWidget () from
/usr/X11R6/lib/libXt.so.6
>#10 0x418fcc3e in _XtSendFocusEvent () from
/usr/X11R6/lib/libXt.so.6
>#11 0x41904fbf in _XtHandleFocus () from
/usr/X11R6/lib/libXt.so.6
>#12 0x418fb76f in XtDispatchEventToWidget () from
/usr/X11R6/lib/libXt.so.6
>#13 0x418fc11d in _XtOnGrabList () from
/usr/X11R6/lib/libXt.so.6
>#14 0x418fc43f in XtDispatchEvent () from
/usr/X11R6/lib/libXt.so.6
>#15 0x080b897e in ServerDispatchEvent
(event=0x419306b0) at server.c:212
>#16 0x080b8884 in ServerMainLoop (context=0x83a4868)
at server.c:170
>#17 0x0805178b in main (argc=2, argv=0xbffff6e4) at
nedit.c:785
>
>It was busy removing some directories where NEdit had
opened
>documents, and closed two of them. A second late I
look back and NEdit
>was frozen solid with all of the CPU it could get. Now
CPU usage is
>back to normal, but NEdit is still frozen.

I'm guessing that the following has happened (in this
order):

- You deleted a file.
- You closed the matching window.
- A focus change event arrived late (asynchronously)
after some
of the widgets were destroyed already.
- The CheckForChangesToFile callback is unaware of
the ongoing
destructions and (indirectly) calls WidgetToWindow.
- WidgetToWindow tries to climb the widget hierarchy
to find
the top widget, but because some of the structures have
been destroyed it accesses garbage and it ends up in an
endless loop. It could have crashed equally well.

I've seen similar things before, but they are almost
impossible to
reproduce (timing is crucial). Most probably, it's our
fault, and not a
bug in X, but I have no idea what is wrong exactly or
how you could
safely detect/avoid accesses to destroyed widgets.

[Eddy De Greef]

Logged In: YES
user_id=73597

My earlier analysis was wrong.

The problem was that, if the user closed the window while a
"File modified externally" dialog was up, we still tried to
update the title of the (destroyed) window. It's now fixed
in CVS.

[Eddy De Greef]

Logged In: YES
user_id=73597
Originator: NO

Reopening this bug, because closing a window/tab from "File modified/File disappeared/..." dialog is still very likely to result in a crash.

The implementation of the CheckForChangesToFile() check is seriously broken: it can be triggered (indirectly) at too many places and almost nowhere it is checked whether the window/document still exists after the call.

Moreover, callbacks are registered with windows as arguments, but it is not guaranteed that a window still exists when the callback is called because the window may have been destroyed during CheckForChangesToFile().

The bugs that I fixed recently were only the tip of the iceberg, I now realize. I think that the problem is particulary nasty for tabbed windows with widgets being shared among documents.

I wish I had the time to have a closer look, but I currently don't.

[Thorsten Haude]

Logged In: YES
user_id=119143
Originator: YES

Let me know if your time table changes. Until then, I have a look.

[Eddy De Greef]

Logged In: YES
user_id=73597
Originator: NO

The (remaining) bug was introduced in version 1.100 of file.c.

The "File modified..." dialogs did not have a Close button before. Closing a document from within this callback is simply not possible in a safe way, especially when it's displayed in a tab. There are too many scenarios that can trigger the dialog, so the Close buttons must be removed again. (I'd prefer this small nuisance over the few crashes a day that I get recently.)

Note that my original fix remains valid: even if we disallow closing a window from within the dialog, it is still possible that the user closes a _window_ through the window manager. (This bug was present in earlier releases.) Apparently, closing a window is safe (provided that we return immediately), because we destroy the complete widget tree and no pending callbacks are called any more.

BTW: the simplest way to trigger a crash/hang:

touch x y
nedit -tabbed x y &
rm x
click on tab x
hit Close in the dialog

[Scott Tringali]

Logged In: YES
user_id=11321
Originator: NO

I agree with Ed. The feature is not worth the stability. Whoever deletes it, please leave a comment saying somethiing like "It would be nice to have a Close button here, but...".

Usually, the way to handle this kind of thing is to enqueue an event for either the callback or the close event, so that it becomes part of the normal time-ordered processing. Doing long, complex, or major changes in callback is usually a bad idea.

Did this bug get released? I thought we only made the change in CVS.

[Eddy De Greef]

Logged In: YES
user_id=73597
Originator: NO

The bug that triggered the original report is present in the latest releases (closing the window via the window manager).
The crashes triggered by the close button are closely related, but are not present in any release. I only realized this yesterday.

[Thorsten Haude]

Logged In: YES
user_id=119143
Originator: YES

I do think it's a little more than a small nuisance, but it's no contest. I will take it out as soon as I find the time.

[Thorsten Haude]

Logged In: YES
user_id=119143
Originator: YES

The attached patch simply removes the Close button from the dialog. Please look out for any further problem.
File Added: noClose.2007-01-03.2.diff

[Thorsten Haude]