Re: [lmi] [PATCH] Add undisplayable_exception

[Vadim Zeitlin]

On Wed, 08 Oct 2014 15:33:35 +0000 Greg Chicares <address@hidden> wrote:

GC> On 2014-10-06 18:08Z, Vadim Zeitlin wrote:
GC> [...]
GC> > +class undisplayable_exception
GC> 
GC> I get a segfault in a case that shouldn't be allowed to arise.

 Sorry, I hadn't thought about the possibility of throwing such exception
from non-testing code. To be honest, I think this is unlikely to happen by
accident, especially if I update the comment to make it more explicit that
this exception should be used only in the testing code. And maybe it should
be renamed to something scarier, e.g. habitually_fatal_exception.

 But if this is not enough, I can think of another, arguably better
solution: instead of modifying report_exception() to know about this
exception, could we make report_exception() itself customizable? E.g.
delegate its work to some function pointer, if it's non-NULL? This would
allow to not mention undisplayable_exception in the main program code at
all and make it local to main_wx_test.cpp and so impossible to throw from
anywhere else -- and, in any case, the main program would report all
exceptions, avoiding the problem entirely, as it would use exactly the same
report_exception() as now.

GC> I applied both your 2014-10-06 patches, along with the following...
GC> 
GC> [BEGIN PATCH]
GC> Index: skeleton.cpp
GC> ===================================================================
GC> --- skeleton.cpp    (revision 5974)
GC> +++ skeleton.cpp    (working copy)
GC> @@ -894,6 +894,7 @@
GC> 
GC>  void Skeleton::UponTestAppStandardException(wxCommandEvent&)
GC>  {
GC> +throw undisplayable_exception("UNDISPLAYABLE");
GC>      throw std::runtime_error("Test a standard exception.");
GC>  }
GC> 
GC> [END PATCH]
GC> ...to find out what would happen if we ever make the mistake of
GC> throwing this new exception from code used in production. Of course,
GC> we should guard against that--perhaps in 'test_coding_rules.cpp', by
GC> forbidding the exception's name in any file not matching '*wx_test*'.
GC> But I wanted to know what would happen.
GC> 
GC> Because this exception is designed to leak through the production
GC> system's exception colander, I kind of hoped for the behavior that
GC> exit() or a std::terminate_handler might give. But instead I got
GC> the segfault below, and it seems curious enough that I thought I
GC> should ask about it.

 I'd say this is more wrong than curious. However this behaviour is
something that changed in wxWidgets 3.1, as part of the work needed to
allow exceptions to propagate through wxYield() that the GUI tests rely on.
So, if it helps, when using 3.1, there is no crash, but the program aborts
instead (i.e. literally calls the standard abort() function). This is still
not ideal, but at least it is more conforming to your expectations.


GC> Perhaps the SendMessage(WM_MDISETMENU) failure for insufficient
GC> storage is merely incidental and uninteresting.

 Strangely enough, I don't see it here:

        % gdb --args ./lmi_wx_shared.exe --data_path=/opt/lmi/data --xxx_yyyy
        Reading symbols from ./lmi_wx_shared.exe...done.
        (gdb) r
        Starting program: /opt/lmi/bin/lmi_wx_shared.exe 
--data_path=/opt/lmi/data --xxx_yyyy
        [New Thread 848.0x52c]
        Program received signal SIGSEGV, Segmentation fault.
        0xfeeefeee in ?? ()

i.e. it just crashes outright, without any warnings. Also, looking slightly
ahead, it doesn't give this message even when using wxWidgets 3.1 (current
trunk), where it doesn't crash (but the MDI code didn't change in 3.1). I
think it might be due to using a different OS, as I'm testing this under
Windows 7, while you are presumably using Windows XP.

 In any case, the crash on its own is already bad news. I've debugged it
and it happens because there is a pending (Windows) message to the main
program window, which is being dispatched when the dialog by
wxSafeShowMessage() called from Skeleton::OnUnhandledException() is about
to be shown. The trouble is that the handler of this message uses
wxDocManager but it had been already destroyed by Skeleton::OnExit().

 Right now I don't see any really obviously good way to fix this, just some
ugly workarounds which I hesitate to suggest, I'll spend some more time
thinking about this and will have to convince myself that there are no
better solutions first.


 In the meanwhile, I'd like to know what do you think about
report_exception() proposal in the beginning of this email which could
allow us to circumvent the problem entirely.


 And I'd also like to ask for the permission to commit the trivial patch
below, which shows the message inside the dialog and not in its title bar
(I realize that caring about such details in a fatal abort message is akin
to rearranging chairs on the desk of a sinking liner, but this is still not
a reason to get them wrong).

 Thanks in advance,
VZ

-- >8 --
diff --git a/skeleton.cpp b/skeleton.cpp
index 19fa351..c89b9bf 100644
--- a/skeleton.cpp
+++ b/skeleton.cpp
@@ -1105,7 +1105,7 @@ void Skeleton::UponTimer(wxTimerEvent&)
 //
 void Skeleton::OnUnhandledException()
 {
-    wxSafeShowMessage("Terminating due to unhandled exception.", "Fatal 
error");
+    wxSafeShowMessage("Fatal error", "Terminating due to unhandled 
exception.");
 }

 // TODO ?? An unsuccessful experiment.