From 2a4a006353890faed3526083f5107dc1c8e4d76f Mon Sep 17 00:00:00 2001 From: Lauri Nurmi Date: Sun, 8 Sep 2024 22:27:13 +0300 Subject: [PATCH 1/2] Extract the actual code from wxTRY blocks into a separate functions Leaving wxTRY blocks with only a few lines of code as-is, but refactoring those with non-trivial amount of codelines into functions. --- include/wx/evtloop.h | 2 + src/common/evtloopcmn.cpp | 177 ++++++++++++++++++++------------------ src/common/init.cpp | 41 +++++---- src/msw/thread.cpp | 33 ++++--- src/unix/threadpsx.cpp | 16 ++-- 5 files changed, 148 insertions(+), 121 deletions(-) diff --git a/include/wx/evtloop.h b/include/wx/evtloop.h index 66307be9870b..81c946094425 100644 --- a/include/wx/evtloop.h +++ b/include/wx/evtloop.h @@ -253,6 +253,8 @@ class WXDLLIMPEXP_BASE wxEventLoopManual : public wxEventLoopBase int m_exitcode; private: + void Loop(); + // process all already pending events and dispatch a new one (blocking // until it appears in the event queue if necessary) // diff --git a/src/common/evtloopcmn.cpp b/src/common/evtloopcmn.cpp index 69a51b3bc335..d19d928e5e6e 100644 --- a/src/common/evtloopcmn.cpp +++ b/src/common/evtloopcmn.cpp @@ -243,6 +243,96 @@ bool wxEventLoopManual::ProcessEvents() return res; } +void wxEventLoopManual::Loop() +{ + // this is the event loop itself + for ( ;; ) + { + // give them the possibility to do whatever they want + OnNextIteration(); + + // generate and process idle events for as long as we don't + // have anything else to do, but stop doing this if Exit() is + // called by one of the idle handlers + // + // note that Pending() only checks for pending events from the + // underlying toolkit, but not our own pending events added by + // QueueEvent(), so we need to call HasPendingEvents() to check + // for them too + while (!m_shouldExit + && !Pending() + && !(wxTheApp && wxTheApp->HasPendingEvents()) + && ProcessIdle()) + ; + + // if Exit() was called, don't dispatch any more events here + if ( m_shouldExit ) + break; + + // a message came or no more idle processing to do, dispatch + // all the pending events and call Dispatch() to wait for the + // next message + if ( !ProcessEvents() || m_shouldExit ) + break; + } + + // Process any still pending events. + for ( ;; ) + { + bool hasMoreEvents = false; + + // We always dispatch events pending at wx level: it may be + // important to do it before the loop exits and e.g. the modal + // dialog possibly referenced by these events handlers is + // destroyed. It also shouldn't result in the problems + // described below for the native events and while there is + // still a risk of never existing the loop due to an endless + // stream of events generated from the user-defined event + // handlers, we consider that well-behaved programs shouldn't + // do this -- and if they do, it's better to keep running the + // loop than crashing after leaving it. + if ( wxTheApp && wxTheApp->HasPendingEvents() ) + { + wxTheApp->ProcessPendingEvents(); + hasMoreEvents = true; + } + + // For the underlying toolkit events, we only handle them when + // exiting the outermost event loop but not when exiting nested + // loops. This is required at least under MSW where, in case of + // a nested modal event loop, the modality has already been + // undone as Exit() had been already called, so all UI elements + // are re-enabled and if we dispatched events from them here, + // we could end up reentering the same event handler that had + // shown the modal dialog in the first place and showing the + // dialog second time before its first instance was destroyed, + // resulting in a lot of fun. + // + // Also, unlike wx events above, it should be fine to dispatch + // the native events from the outer event loop, as any events + // generated from outside the dialog itself (necessarily, as + // the dialog is already hidden and about to be destroyed) + // shouldn't reference the dialog. Which is one of the reasons + // we still dispatch them in the outermost event loop, to + // ensure they're still processed. Another reason is that if we + // do have an endless stream of native events, e.g. because we + // have a timer with a too short interval, it's arguably better + // to keep handling them instead of exiting. + if ( gs_eventLoopCount == 1 ) + { + if ( Pending() ) + { + Dispatch(); + hasMoreEvents = true; + } + } + + if ( !hasMoreEvents ) + break; + } + +} + int wxEventLoopManual::DoRun() { @@ -257,92 +347,7 @@ int wxEventLoopManual::DoRun() try { #endif // wxUSE_EXCEPTIONS - - // this is the event loop itself - for ( ;; ) - { - // give them the possibility to do whatever they want - OnNextIteration(); - - // generate and process idle events for as long as we don't - // have anything else to do, but stop doing this if Exit() is - // called by one of the idle handlers - // - // note that Pending() only checks for pending events from the - // underlying toolkit, but not our own pending events added by - // QueueEvent(), so we need to call HasPendingEvents() to check - // for them too - while ( !m_shouldExit - && !Pending() - && !(wxTheApp && wxTheApp->HasPendingEvents()) - && ProcessIdle() ) - ; - - // if Exit() was called, don't dispatch any more events here - if ( m_shouldExit ) - break; - - // a message came or no more idle processing to do, dispatch - // all the pending events and call Dispatch() to wait for the - // next message - if ( !ProcessEvents() || m_shouldExit ) - break; - } - - // Process any still pending events. - for ( ;; ) - { - bool hasMoreEvents = false; - - // We always dispatch events pending at wx level: it may be - // important to do it before the loop exits and e.g. the modal - // dialog possibly referenced by these events handlers is - // destroyed. It also shouldn't result in the problems - // described below for the native events and while there is - // still a risk of never existing the loop due to an endless - // stream of events generated from the user-defined event - // handlers, we consider that well-behaved programs shouldn't - // do this -- and if they do, it's better to keep running the - // loop than crashing after leaving it. - if ( wxTheApp && wxTheApp->HasPendingEvents() ) - { - wxTheApp->ProcessPendingEvents(); - hasMoreEvents = true; - } - - // For the underlying toolkit events, we only handle them when - // exiting the outermost event loop but not when exiting nested - // loops. This is required at least under MSW where, in case of - // a nested modal event loop, the modality has already been - // undone as Exit() had been already called, so all UI elements - // are re-enabled and if we dispatched events from them here, - // we could end up reentering the same event handler that had - // shown the modal dialog in the first place and showing the - // dialog second time before its first instance was destroyed, - // resulting in a lot of fun. - // - // Also, unlike wx events above, it should be fine to dispatch - // the native events from the outer event loop, as any events - // generated from outside the dialog itself (necessarily, as - // the dialog is already hidden and about to be destroyed) - // shouldn't reference the dialog. Which is one of the reasons - // we still dispatch them in the outermost event loop, to - // ensure they're still processed. Another reason is that if we - // do have an endless stream of native events, e.g. because we - // have a timer with a too short interval, it's arguably better - // to keep handling them instead of exiting. - if ( gs_eventLoopCount == 1 ) - { - if ( Pending() ) - { - Dispatch(); - hasMoreEvents = true; - } - } - - if ( !hasMoreEvents ) - break; - } + Loop(); #if wxUSE_EXCEPTIONS // exit the outer loop as well break; diff --git a/src/common/init.cpp b/src/common/init.cpp index 47635cb8df5b..34446d1b6b21 100644 --- a/src/common/init.cpp +++ b/src/common/init.cpp @@ -528,6 +528,28 @@ void wxAddEntryHook(wxEntryHook hook) #define wxEntryReal wxEntry #endif // !__WINDOWS__ +static int DoEntryReal() +{ + // app initialization + if ( !wxTheApp->CallOnInit() ) + { + // don't call OnExit() if OnInit() failed + return wxTheApp->GetErrorExitCode(); + } + + // ensure that OnExit() is called if OnInit() had succeeded + class CallOnExit + { + public: + ~CallOnExit() { wxTheApp->OnExit(); } + } callOnExit; + + WX_SUPPRESS_UNUSED_WARN(callOnExit); + + // app execution + return wxTheApp->OnRun(); +} + int wxEntryReal(int& argc, wxChar **argv) { // Do this before trying the hooks as they may use command line arguments. @@ -555,24 +577,7 @@ int wxEntryReal(int& argc, wxChar **argv) wxTRY { - // app initialization - if ( !wxTheApp->CallOnInit() ) - { - // don't call OnExit() if OnInit() failed - return wxTheApp->GetErrorExitCode(); - } - - // ensure that OnExit() is called if OnInit() had succeeded - class CallOnExit - { - public: - ~CallOnExit() { wxTheApp->OnExit(); } - } callOnExit; - - WX_SUPPRESS_UNUSED_WARN(callOnExit); - - // app execution - return wxTheApp->OnRun(); + return DoEntryReal(); } wxCATCH_ALL( wxTheApp->OnUnhandledException(); diff --git a/src/msw/thread.cpp b/src/msw/thread.cpp index df1d343e3ca0..074b68fc5f44 100644 --- a/src/msw/thread.cpp +++ b/src/msw/thread.cpp @@ -452,6 +452,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 DoThreadStartWithoutExceptionHandling(wxThread *thread); + // call OnExit() on the thread static void DoThreadOnExit(wxThread *thread); @@ -507,6 +510,23 @@ void wxThreadInternal::DoThreadOnExit(wxThread *thread) wxCATCH_ALL( wxTheApp->OnUnhandledException(); ) } +/* static */ +THREAD_RETVAL wxThreadInternal::DoThreadStartWithoutExceptionHandling(wxThread* thread) +{ + // store the thread object in the TLS + wxASSERT_MSG(gs_tlsThisThread != TLS_OUT_OF_INDEXES, + "TLS index not set. Is wx initialized?"); + + if (!::TlsSetValue(gs_tlsThisThread, thread)) + { + wxLogSysError(_("Cannot start thread: error writing TLS.")); + + return THREAD_ERROR_EXIT; + } + + return wxPtrToUInt(thread->Entry()); +} + /* static */ THREAD_RETVAL wxThreadInternal::DoThreadStart(wxThread *thread) { @@ -516,18 +536,7 @@ THREAD_RETVAL wxThreadInternal::DoThreadStart(wxThread *thread) wxTRY { - // store the thread object in the TLS - wxASSERT_MSG( gs_tlsThisThread != TLS_OUT_OF_INDEXES, - "TLS index not set. Is wx initialized?" ); - - if ( !::TlsSetValue(gs_tlsThisThread, thread) ) - { - wxLogSysError(_("Cannot start thread: error writing TLS.")); - - return THREAD_ERROR_EXIT; - } - - rc = wxPtrToUInt(thread->Entry()); + rc = DoThreadStartWithoutExceptionHandling(thread); } wxCATCH_ALL( wxTheApp->OnUnhandledException(); ) diff --git a/src/unix/threadpsx.cpp b/src/unix/threadpsx.cpp index 13dbda1d7baf..561ade9d520d 100644 --- a/src/unix/threadpsx.cpp +++ b/src/unix/threadpsx.cpp @@ -793,6 +793,16 @@ class wxThreadInternal #endif // wxHAVE_PTHREAD_CLEANUP private: + static void CallThreadEntryWithoutExceptionHandling(wxThreadInternal *pthread, + wxThread *thread) + { + pthread->m_exitcode = thread->Entry(); + + wxLogTrace(TRACE_THREADS, + wxT("Thread %p Entry() returned %lu."), + THR_ID(pthread), wxPtrToUInt(pthread->m_exitcode)); + } + pthread_t m_threadId; // id of the thread wxThreadState m_state; // see wxThreadState enum int m_prio; // in wxWidgets units: from 0 to 100 @@ -881,11 +891,7 @@ void *wxThreadInternal::PthreadStart(wxThread *thread) wxTRY { - pthread->m_exitcode = thread->Entry(); - - wxLogTrace(TRACE_THREADS, - wxT("Thread %p Entry() returned %lu."), - THR_ID(pthread), wxPtrToUInt(pthread->m_exitcode)); + CallThreadEntryWithoutExceptionHandling(pthread, thread); } #ifndef wxNO_EXCEPTIONS #ifdef HAVE_ABI_FORCEDUNWIND From c8980d531e83dabff5c278c4e4b35eb13d7fe65f Mon Sep 17 00:00:00 2001 From: Lauri Nurmi Date: Sun, 8 Sep 2024 22:35:29 +0300 Subject: [PATCH 2/2] Allow opting out of catching unhandled C++ exceptions 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 --- interface/wx/sysopt.h | 9 +++++++++ src/common/event.cpp | 5 +++++ src/common/evtloopcmn.cpp | 6 ++++++ src/common/init.cpp | 5 +++++ src/msw/thread.cpp | 17 ++++++++++++----- src/unix/threadpsx.cpp | 11 +++++++++++ 6 files changed, 48 insertions(+), 5 deletions(-) diff --git a/interface/wx/sysopt.h b/interface/wx/sysopt.h index 2fc3bdd2dc7e..d53c3dd8a426 100644 --- a/interface/wx/sysopt.h +++ b/interface/wx/sysopt.h @@ -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} + If set to zero, wxWidgets will not catch unhandled exceptions, but + rather lets the default behavior of aborting the program take place. + Not catching unhandled exceptions makes debugging easier, as the + backtrace is more likely to show what actually happened, and where. + The same applies to any crash dumps generated due to unhandled exceptions. + By default unhandled exceptions are eventually caught by wxWidgets. + This flag should be set very early during program startup, within + the constructor of the wxApp derivative. @endFlagTable @section sysopt_win Windows diff --git a/src/common/event.cpp b/src/common/event.cpp index 09a8029a550f..22a590832bc4 100644 --- a/src/common/event.cpp +++ b/src/common/event.cpp @@ -22,6 +22,7 @@ #include "wx/event.h" #include "wx/eventfilter.h" #include "wx/evtloop.h" +#include "wx/sysopt.h" #ifndef WX_PRECOMP #include "wx/list.h" @@ -1666,6 +1667,10 @@ bool wxEvtHandler::TryHereOnly(wxEvent& event) bool wxEvtHandler::SafelyProcessEvent(wxEvent& event) { + if ( wxSystemOptions::IsFalse("catch-unhandled-exceptions") ) + { + return ProcessEvent(event); + } #if wxUSE_EXCEPTIONS try { diff --git a/src/common/evtloopcmn.cpp b/src/common/evtloopcmn.cpp index d19d928e5e6e..9543e5a65f40 100644 --- a/src/common/evtloopcmn.cpp +++ b/src/common/evtloopcmn.cpp @@ -20,6 +20,7 @@ #include "wx/scopeguard.h" #include "wx/apptrait.h" +#include "wx/sysopt.h" #include "wx/private/eventloopsourcesmanager.h" // Counts currently existing event loops. @@ -342,6 +343,11 @@ int wxEventLoopManual::DoRun() // wxModalEventLoop depends on this (so we can't just use ON_BLOCK_EXIT or // something similar here) #if wxUSE_EXCEPTIONS + if ( wxSystemOptions::IsFalse("catch-unhandled-exceptions") ) + { + Loop(); + return m_exitcode; + } for ( ;; ) { try diff --git a/src/common/init.cpp b/src/common/init.cpp index 34446d1b6b21..859ce19a8b54 100644 --- a/src/common/init.cpp +++ b/src/common/init.cpp @@ -29,6 +29,7 @@ #include "wx/atomic.h" #include "wx/except.h" +#include "wx/sysopt.h" #if defined(__WINDOWS__) #include "wx/msw/private.h" @@ -575,6 +576,10 @@ int wxEntryReal(int& argc, wxChar **argv) return wxApp::GetFatalErrorExitCode(); } + if ( wxSystemOptions::IsFalse("catch-unhandled-exceptions") ) + { + return DoEntryReal(); + } wxTRY { return DoEntryReal(); diff --git a/src/msw/thread.cpp b/src/msw/thread.cpp index 074b68fc5f44..88cf7359523e 100644 --- a/src/msw/thread.cpp +++ b/src/msw/thread.cpp @@ -38,6 +38,7 @@ #include "wx/except.h" #include "wx/dynlib.h" +#include "wx/sysopt.h" // must have this symbol defined to get _beginthread/_endthread declarations #ifndef _MT @@ -503,9 +504,13 @@ class wxThreadKeepAlive /* static */ void wxThreadInternal::DoThreadOnExit(wxThread *thread) { + if ( wxSystemOptions::IsFalse("catch-unhandled-exceptions") ) + { + return thread->OnExit(); + } wxTRY { - thread->OnExit(); + return thread->OnExit(); } wxCATCH_ALL( wxTheApp->OnUnhandledException(); ) } @@ -532,15 +537,17 @@ THREAD_RETVAL wxThreadInternal::DoThreadStart(wxThread *thread) { wxON_BLOCK_EXIT1(DoThreadOnExit, thread); - THREAD_RETVAL rc = THREAD_ERROR_EXIT; - + if ( wxSystemOptions::IsFalse("catch-unhandled-exceptions") ) + { + return DoThreadStartWithoutExceptionHandling(thread); + } wxTRY { - rc = DoThreadStartWithoutExceptionHandling(thread); + return DoThreadStartWithoutExceptionHandling(thread); } wxCATCH_ALL( wxTheApp->OnUnhandledException(); ) - return rc; + return THREAD_ERROR_EXIT; } /* static */ diff --git a/src/unix/threadpsx.cpp b/src/unix/threadpsx.cpp index 561ade9d520d..92bcc6678a14 100644 --- a/src/unix/threadpsx.cpp +++ b/src/unix/threadpsx.cpp @@ -27,6 +27,7 @@ #include "wx/thread.h" #include "wx/except.h" +#include "wx/sysopt.h" #ifndef WX_PRECOMP #include "wx/app.h" @@ -889,6 +890,11 @@ void *wxThreadInternal::PthreadStart(wxThread *thread) wxT("Thread %p about to enter its Entry()."), THR_ID(pthread)); + if ( wxSystemOptions::IsFalse("catch-unhandled-exceptions") ) + { + CallThreadEntryWithoutExceptionHandling(pthread, thread); + } + else wxTRY { CallThreadEntryWithoutExceptionHandling(pthread, thread); @@ -1740,6 +1746,11 @@ void wxThread::Exit(ExitCode status) // might deadlock if, for example, it signals a condition in OnExit() (a // common case) while the main thread calls any of functions entering // m_critsect on us (almost all of them do) + if ( wxSystemOptions::IsFalse("catch-unhandled-exceptions") ) + { + OnExit(); + } + else wxTRY { OnExit();