Allow opting out of catching unhandled C++ exceptions (PR #24997)
Lauri Nurmi
Add a system option to control whether the main loop and threads are run
from within a try-catch block, or outside of it.
Traditionally, wxWidgets has caught all unhandled C++ exceptions
that are about to cause the program to be aborted.
For many situations this is probably a good thing, but the downside
is that it severely complicates debugging the source of such exceptions.
The backtrace will only show the stack since wx's exception handler,
but not where the exception occurred in the first place.
Any crashdumps created by the OS also suffer from the same issue;
backtraces are mostly useless in these cases.
The following classes with try-catch blocks do not obey the new system
option for now: wxDataViewRendererBase, wxDocTemplate, wxIDropTarget
Attached here is a patch for triggering a crash (abort) from a sample, to help demonstrating what happens with the new option set or not set.
Minimal sample: Help -> About
Threads sample: Thread -> Start a new thread
You can view, comment on, or merge this pull request online at:
https://github.com/wxWidgets/wxWidgets/pull/24997
Commit Summary
- ad85670 Extract the actual code from wxTRY blocks into a separate functions
- f0e0a76 Allow opting out of catching unhandled C++ exceptions
File Changes
(7 files)
- M include/wx/evtloop.h (2)
- M interface/wx/sysopt.h (9)
- M src/common/event.cpp (6)
- M src/common/evtloopcmn.cpp (185)
- M src/common/init.cpp (47)
- M src/msw/thread.cpp (44)
- M src/unix/threadpsx.cpp (15)
Patch Links:
- https://github.com/wxWidgets/wxWidgets/pull/24997.patch
- https://github.com/wxWidgets/wxWidgets/pull/24997.diff
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
@vadz commented on this pull request.
Thanks, I agree that it can be useful to disable built-in exception handling in some situations, but I have a few questions about the details.
> + if ( wxSystemOptions::IsFalse("catch-unhandled-exceptions") )
+ {
+ Loop();
+ // exit the outer loop as well
+ break;
+ }
+ else
This looks weird to me, why do we put this inside a loop only to break out of it unconditionally? Why not just put
if ( ...propagate exceptions... ) { Loop(); return m_exitcode; }
before the loop?
> @@ -553,26 +576,14 @@ int wxEntryReal(int& argc, wxChar **argv)
return wxApp::GetFatalErrorExitCode();
}
+ if ( wxSystemOptions::IsFalse("catch-unhandled-exceptions") )
+ {
+ return DoEntryReal();
+ }
+ else
Here I'd just remove this else.
> @@ -500,34 +504,50 @@ class wxThreadKeepAlive
/* static */
void wxThreadInternal::DoThreadOnExit(wxThread *thread)
{
+ if ( wxSystemOptions::IsFalse("catch-unhandled-exceptions") )
+ {
+ thread->OnExit();
+ }
+ else
And remove this one too.
> @@ -452,6 +453,9 @@ class wxThreadInternal
// really start the thread (if it's not already dead)
static THREAD_RETVAL DoThreadStart(wxThread *thread);
+ // really really start the thread, without catching exceptions
+ static THREAD_RETVAL DoDoThreadStart(wxThread *thread);
Sorry but I really hate these "DoDo" names. Let's call it DoThreadStartWithoutExceptionHandling() or DoThreadStartWithoutEH() or even DoThreadStartNoEH() if it's too long.
Alternatively we could avoid having to come up with a name for it by using a lambda inside the function.
> @@ -37,6 +37,15 @@
this option allows changing it without modifying the program code and
also applies to asserts which may happen before the wxApp object
creation or after its destruction.
+ @flag{catch-unhandled-exceptions}
I don't have a strong opinion about this, but it seems a bit strange to have an option which is 1 by default. Maybe we should have allow-exceptions-escape or something like that with the opposite meaning instead?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Lauri Nurmi
@lanurmi commented on this pull request.
> @@ -37,6 +37,15 @@
this option allows changing it without modifying the program code and
also applies to asserts which may happen before the wxApp object
creation or after its destruction.
+ @flag{catch-unhandled-exceptions}
I don't think it is that strange. IMO nothing in system options' documentation or the name itself suggests that options are inherently 0 by default. (Also cf. Cocoa's NSUserDefaults). In fact, there are two existing system options that are 1 by default: msw.notebook.themed-background and msw.staticbox.optimized-paint.
Enforcing 0-by-default is bound to lead to more or less awkward naming with made up opposite meanings. E.g. in allow-exceptions-escape, 'escape' is not a term that the C++ standard uses with exceptions at all.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Lauri Nurmi
@lanurmi commented on this pull request.
> @@ -452,6 +453,9 @@ class wxThreadInternal
// really start the thread (if it's not already dead)
static THREAD_RETVAL DoThreadStart(wxThread *thread);
+ // really really start the thread, without catching exceptions
+ static THREAD_RETVAL DoDoThreadStart(wxThread *thread);
I did think about lambdas, but trying to keep the modifications potentially 3.2-backportable. Will rename.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Lauri Nurmi
> @@ -500,34 +504,50 @@ class wxThreadKeepAlive
/* static */
void wxThreadInternal::DoThreadOnExit(wxThread *thread)
{
+ if ( wxSystemOptions::IsFalse("catch-unhandled-exceptions") )
+ {
+ thread->OnExit();
+ }
+ else
But then thread->OnExit() would get called twice.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Lauri Nurmi
@lanurmi commented on this pull request.
> + if ( wxSystemOptions::IsFalse("catch-unhandled-exceptions") )
+ {
+ Loop();
+ // exit the outer loop as well
+ break;
+ }
+ else
Can be done.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Lauri Nurmi
@lanurmi pushed 3 commits.
- c18d26a fixup! Extract the actual code from wxTRY blocks into a separate functions
- 7b2f213 fixup! Extract the actual code from wxTRY blocks into a separate functions
- 76c449c fixup! Allow opting out of catching unhandled C++ exceptions
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
Lauri Nurmi
@lanurmi pushed 1 commit.
- dc46b72 fixup! Allow opting out of catching unhandled C++ exceptions
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
@vadz commented on this pull request.
> @@ -37,6 +37,15 @@
this option allows changing it without modifying the program code and
also applies to asserts which may happen before the wxApp object
creation or after its destruction.
+ @flag{catch-unhandled-exceptions}
We could use disable-catch-unhandled.
Anyhow, as I said, I don't feel that strongly about this, so if you do, we can leave it like this but I'd still be curious if nobody else really finds this awkward, as I do.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.