[lmi] Re[3]: crash in wxFileHistory code (was: Untimely "save" dialog)

[Vadim Zeitlin]

On Mon, 9 Mar 2009 18:08:58 +0100 Vadim Zeitlin <address@hidden> wrote:

VZ> GC> Now I observe a sporadic crash in a similar situation.
VZ> 
VZ>  Unfortunately I couldn't reproduce the crash. It might be due to a
VZ> difference in exception handling between MSVC and mingw32 but to be honest
VZ> I don't see how is this possible. Looking at the backtrace I don't quite
VZ> understand what's going on neither, it looks like m_fileMenus in
VZ> wxFileHistory got corrupted somehow as otherwise I don't see why would the
VZ> call to SetLabel() crash.

 For once I guessed the cause of the crash almost correctly: the list
itself wasn't corrupted but it did contain the already deleted pointers.
What happens is that ViewEx::OnCreate() calls DocManager().
AssociateFileHistoryWithFileMenu() and normally OnClose() executes the
matching DissociateFileHistoryFromFileMenu() call. But if an exception is
thrown from IllustrationView::Run(), OnClose() is never called and so the
menu pointer remains in the list of the menus managed by wxFileHistory even
though it's going to be deleted as part of the frame destruction process.
This explains that more you try, more is the probability of reproducing the
crash as there are more and more invalid pointers in the list. But other
than that it seems like it was just pure (bad) luck that I couldn't
reproduce it before. And I'm also not sure that the problem is specific to
2.9 but I didn't check this.

 Anyhow, the more interesting question is surely how to fix this. It would
be nice if disassociating menus from the file history could be done by wx
automatically but this doesn't seem to be simple to do because there is
currently no way to be modified about a menu destruction. So it seems that
this should be worked around at LMI level. The simplest solution would be
to catch exceptions thrown by Run() in IllustrationView::OnCreate() and
call DissociateFileHistoryFromFileMenu() inside the catch clause (before
re-throwing the exception) but I don't really like it because of the
asymmetry it introduces: AssociateFileHistoryWithFileMenu() is called from
inside ViewEx class while the clean up function must be called manually
from outside it. So I'd probably do one of the following instead:

1. Call DissociateFileHistoryFromFileMenu() in ViewEx dtor if this hadn't
   been done before -- this seems like the safest method

2. Don't call AssociateFileHistoryWithFileMenu() in the base class
   OnCreate() but only do it in the derived class just before returning
   true. Maybe this could be done in addition to (1). If we refactor
   ViewEx::OnCreate() I'd also like to avoid calling GetFrame()->Show() in
   it as this really should be done only once everything is constructed and
   initialized, i.e. after Run() returns.

Would you like me to prepare a patch doing either (1) or (2) or some better
solution that you see or would you prefer to fix this yourself?

 Thanks,
VZ