fix: call the original terminate handler even after monitors are disabled - #8663
fix: call the original terminate handler even after monitors are disabled#8663devunt wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Thank you for your contribution and the detailed debugging it took to find the source of the problem! Your analysis looks spot on, however I think the fix can be simpler - removing g_originalTerminateHandler = NULL; in setEnabled() would be a more correct fix since capturing the handler during an exception would miss cases where the monitor is enabled then disabled before an exception is handled.
Edit for maintainers: looking at upstream KSCrash - the monitor maintains it's handlers always and only runs it's logic if enabled so it doesn't suffer this issue.
…n monitor is disabled For fatal C++ exceptions, CPPExceptionTerminate() calls sentrycrashcm_handleException(), which disables all monitors and thereby runs this monitor's setEnabled(false), resetting g_originalTerminateHandler to NULL. The subsequent sentrycrashcm_cppexception_callOriginalTerminationHandler() then becomes a no-op, so any terminate handler installed before the SDK (e.g. Kotlin/Native's unhandled-exception reporter) never runs, and libc++abi aborts with "terminate_handler unexpectedly returned". The same applies when an exception is thrown while the monitor is enabled but the monitor is disabled before the exception terminates the process: libc++abi stores the terminate handler per exception at throw time, so CPPExceptionTerminate can still run after setEnabled(false). Keep g_originalTerminateHandler when disabling the monitor, matching upstream KSCrash behavior. setEnabled(true) overwrites it anyway, and callOriginalTerminationHandler() already guards against NULL for the never-enabled case.
d2a1853 to
ce7f233
Compare
|
Thanks for the review, @NinjaLikesCheez — agreed, your version is more correct. The entry-time capture misses the case you describe: since libc++abi stores the terminate handler per exception at throw time ( Rewrote the PR accordingly:
|
Thank you! Kicking off the CI now :) |
📜 Description
For fatal C++ exceptions,
CPPExceptionTerminate()never invokes the terminate handler that was installed before the SDK, and the process instead aborts with libc++abi's"terminate_handler unexpectedly returned".The chain:
CPPExceptionTerminate()writes the crash report viasentrycrashcm_handleException().sentrycrashcm_handleException()callssentrycrashcm_setActiveMonitors(SentryCrashMonitorTypeNone)("Exception is fatal. Restoring original handlers.").setEnabled(false), which restoresstd::set_terminate(g_originalTerminateHandler)and resetsg_originalTerminateHandlertoNULL.CPPExceptionTerminate(), which then callssentrycrashcm_cppexception_callOriginalTerminationHandler()— now a guaranteed no-op because the global isNULL.CPPExceptionTerminate()returns into libc++abi'sstd::__terminate, which aborts with"terminate_handler unexpectedly returned"(cxa_handlers.cpp).The fix removes the
g_originalTerminateHandler = NULL;reset insetEnabled(false), sosentrycrashcm_cppexception_callOriginalTerminationHandler()still invokes the pre-existing handler chain after the monitor teardown in step 3. This matches upstream KSCrash, which keeps its handlers and only gates its logic on the enabled flag.Keeping the handler also covers the case an entry-time capture would miss: libc++abi stores the terminate handler per exception at throw time, so
CPPExceptionTerminatecan run for an exception thrown while the monitor was enabled even if the monitor was disabled before the exception terminated the process — at that point the global would already beNULLat function entry. Re-enabling overwrites the handler insetEnabled(true), and the existingNULLguard incallOriginalTerminationHandler()still covers the never-enabled case.💡 Motivation and Context
Any runtime that installs its own
std::terminatehandler beforeSentrySDK.startsilently loses it for fatal C++ exceptions. The concrete case where we found this: Kotlin Multiplatform apps on iOS (sentry-kotlin-multiplatform).Kotlin/Native installs a terminate handler at runtime init that detects unhandled Kotlin exceptions (
ExceptionObjHolder) and runskotlin.native.setUnhandledExceptionHook— which is exactly where the sentry-kotlin-multiplatform SDK hooks in to capture the full Kotlin stack trace and tag the event sodropKotlinCrashEventcan dedup the opaque native report on next launch. Because of this bug, that handler never runs when a Kotlin exception reachesstd::terminatethrough foreign (ObjC/C++) frames — e.g. any exception thrown inside Compose Multiplatform's render loop. The result in production: only an opaqueC++ Exception: N12_GLOBAL__N_122ExceptionObjHolderImplE: (null)event, no Kotlin stack trace, no dedup (related context: getsentry/sentry-kotlin-multiplatform#476).Apple crash report from production (sentry-cocoa 8.58.2, identical code path on
main) showing step 5 — the terminate handler chain returned instead of the original handler aborting:(cxa_handlers.cpp:61 is
abort_message("terminate_handler unexpectedly returned"). Kotlin/Native's handler cannot be the returner: all of its paths end inabort()/_Exit().)💚 How did you test it?
mainshare the same logic) plus the production Apple crash report above.CPPExceptionTerminateentry) and pinned it in the affected app; a forced render-loop Kotlin exception on a device is our follow-up validation.CPPExceptionTerminateis astatichandler on a process-terminating path, and the existing tests only exercise the publicsentrycrashcm_cppexception_callOriginalTerminationHandler()(whose behavior is unchanged).📝 Checklist
You have to check all boxes before merging:
sendDefaultPIIis enabled.