Re[2]: [lmi] Random abend reported

[Vadim Zeitlin]

On Sat, 10 Dec 2005 02:56:54 +0000 Greg Chicares <address@hidden> wrote:

GC> On 2005-12-9 0:53 UTC, Greg Chicares wrote:
GC> > Vadim--I'm trying to track down an abnormal termination
GC> [...]
GC> > It seems to occur when lmi is closed while it has an MDI child
GC> > window open. This seems to have arisen about the time I did this:
GC> >
GC> > 
http://savannah.nongnu.org/cgi-bin/viewcvs/lmi/lmi/main_wx.cpp.diff?r1=1.26&r2=1.27
GC> >
GC> > and I wonder whether you could look at that change and tell me if
GC> > I did something that looks wrong.

 Unfortunately not only I don't see anything wrong but I don't really
understand how is it related to the issue above at all to be honest.

GC> Now, that message from wx:
GC>   "class still has open windows."
GC> seems closely related to the defect report:
GC> > It seems to occur when lmi is closed while it has an MDI child window 
open.
GC> so I thought I'd try to prevent it.

 This message is given when the window hasn't been destroyed (although it
could have been closed) before wxApp is destroyed so it can indeed be
related. I seem to remember that I fixed something like this a long time
ago in wx by just ensuring that the objects pending for deletion are
effectively cleaned up before the classes are unregistered.

GC> With '--test', we return false. Skeleton::OnExit() does perform some
GC> cleanup. But even if that cleanup doesn't happen, the worst consequence
GC> I'd expect is a memory leak, not a crash.

 There shouldn't be any memory leaks neither. And surely no crashes.

GC> I wondered whether I should call wxApp::OnInit explicitly in the application
GC> class's OnInit.

 This is unrelated but I'd answer it anyhow: currently, you should call
base class OnInit() if you rely on wx semi-automatic command line arguments
handling, i.e. if you use OnInitCmdLine() and OnCmdLineXXX(). If you
don't call base class OnInit(), these functions wouldn't work. But more
generally it's a good idea to always call the base class function (and
return false if it returns false: our initialization can't succeed if the
base class failed).

GC> I tried calling it thus:
GC> 
GC>   bool Skeleton::OnInit()
GC>   {
GC>       wxApp::OnInit();
GC>       ...
GC> 
GC> but that invoked the wx parser, and I'm using GNU getopt instead. I conclude
GC> that I shouldn't do that.

 Currently you can indeed just skip calling base class OnInit(). But it's
possible that other tasks are done in it in the future so I think it would
be better to still call it. OTOH for this to work currently you have to
override all OnCmdLineXXX() virtual methods in wxApp to do nothing which is
a bit ugly. It would be better if OnInitCmdLine() had return value as well
and could return, say, -1 to prevent the command line from being parsed at
all but this is a backwards incompatible change as it would silently (i.e.
without compilation errors or even warnings with many compilers) break
run-time behaviour of the existing code. The only solution I see is to add
some new "int OnCmdLineInit() { OnInitCmdLine(); return 1; }" which you
could then override to return -1 in lmi.


GC> That doesn't solve the "class still has open windows" problem.

 This should be solved by using wx 2.6.2. I realize that I keep saying it
all the time but what can I do...


GC> Looking through the wxApp member functions, I thought of trying this:
GC> 
GC>           // This doesn't prevent that message either:
GC>   //        ExitMainLoop();
GC> 
GC> which doesn't solve the problem at hand.

 There is no main loop to exit in this case (it's only entered by OnRun()
if OnInit() returned true).

GC> However:
GC> 
GC>           // But the wx documentation for ExitMainLoop() says "You
GC>           // should normally exit the main loop (and the application)
GC>           // by deleting the top window.
GC>   //        delete GetTopWindow();
GC>           // That does seem to work, but explicitly to delete a window
GC>           // managed by wx seems strange. And the wx documentation says
GC>           // "When deleting a frame or dialog, use Destroy rather than
GC>           // delete", so instead try:
GC>   //        GetTopWindow()->Destroy();
GC>           // That seems to work as well. However, the wx documentation
GC>           // elsewhere suggests using Close() instead: "The advantage of
GC>           // using Close instead of Destroy is that it will call any
GC>           // clean-up code defined by the EVT_CLOSE handler; for example
GC>           // it may close a document contained in a window after first
GC>           // asking the user whether the work should be saved.
GC>           GetTopWindow()->Close();
GC>           // That seems to work as well, and seems best of all according
GC>           // to the wx documentation.

 Yes, this is indeed the safest thing to do. Current wx code does this in
wxAppBase::CleanUp().

GC> Have I analyzed this correctly and chosen the best solution?

 Yes, your solution is a good one and it won't hurt even if/when you update
to latest wx version although I think it will become unnecessary then.

 Regards,
VZ