From 2c117a5a058e36eee43839b87456581101ea79e9 Mon Sep 17 00:00:00 2001 From: Daniel Weindl Date: Tue, 18 Feb 2025 08:51:47 +0100 Subject: [PATCH] Fix crashes due to uncaught exceptions in `~FinalStateStorer` (#2647) Due to https://github.com/LLNL/sundials/issues/82, we can get exceptions from `CVodeSolver::getSens`, escaping `FinalStateStorer::~FinalStateStorer`, that will result in crashes. This is fixed here. The issue is semi-producible with [Boehm_JProteomeRes2014](https://github.com/Benchmarking-Initiative/Benchmark-Models-PEtab/tree/15b4f78aff1121870537ad3e1a6b902cb8f73930/Benchmark-Models/Boehm_JProteomeRes2014) with forward sensitivities: Program output: ``` 2025-02-17 14:44:33.988 - amici.swig_wrappers - DEBUG - [model1_data1][cvodes:CVode:WARNING] python/sdist/amici/ThirdParty/sundials/src/cvodes/cvodes.c:3482: Internal t = 0 and h = 0 are such that t + h = t on the next step. The solver will continue anyway. (99) 2025-02-17 14:44:33.988 - amici.swig_wrappers - DEBUG - [model1_data1][cvodes:CVodeGetDky:BAD_T] python/sdist/amici/ThirdParty/sundials/src/cvodes/cvodes.c:3783: Illegal value for t.t = 2.5 is not between tcur - hu = 0 and tcur = 0. (-25) 2025-02-17 14:44:33.988 - amici.swig_wrappers - DEBUG - [model1_data1][cvodes:CVodeGetDky:BAD_T] python/sdist/amici/ThirdParty/sundials/src/cvodes/cvodes.c:3783: Illegal value for t.t = 5 is not between tcur - hu = 0 and tcur = 0. (-25) 2025-02-17 14:44:33.988 - amici.swig_wrappers - DEBUG - [model1_data1][cvodes:CVode:ILL_INPUT] python/sdist/amici/ThirdParty/sundials/src/cvodes/cvodes.c:3328: Trouble interpolating at tout = 5. tout too far back in direction of integration (-22) 2025-02-17 14:44:33.988 - amici.swig_wrappers - ERROR - [model1_data1][FORWARD_FAILURE] AMICI forward simulation failed at t = 5: AMICI failed to integrate the forward problem terminate called after throwing an instance of 'amici::CvodeException' what(): CVODE routine CVodeGetSens failed with error code -25. Thread 1 "python" received signal SIGABRT, Aborted. ``` Stack trace: ``` #10 0x00007ffff70811f1 in _Unwind_RaiseException (exc=0x6d5ff10) at ../../../src/libgcc/unwind.inc:136 #11 0x00007ffff48bb384 in __cxxabiv1::__cxa_throw (obj=, tinfo=0x7fff5c6d6aa8 , dest=0x7fff5c5476c4 ) at ../../../../src/libstdc++-v3/libsupc++/eh_throw.cc:93 #12 0x00007fff5c5bdb64 in amici::CVodeSolver::getSens (this=0x6d130d0) at python/sdist/amici/src/solver_cvodes.cpp:663 #13 0x00007fffbc5709e8 in amici::Solver::getStateSensitivity (this=0x6d130d0, t=2.5) at python/sdist/amici/src/solver.cpp:1340 #14 0x00007fffbc570750 in amici::Solver::writeSolution (this=0x6d130d0, t=0x6d26db0, x=..., dx=..., sx=..., xQ=...) at python/sdist/amici/src/solver.cpp:1303 #15 0x00007fffbc5d376d in amici::ForwardProblem::getSimulationState (this=0x6d26c90) at python/sdist/amici/src/forwardproblem.cpp:421 #16 0x00007fffbc5d45df in amici::FinalStateStorer::~FinalStateStorer (this=0x7fffffffb308, __in_chrg=) at include/amici/forwardproblem.h:450 #17 0x00007fffbc5d2285 in amici::ForwardProblem::workForwardProblem (this=0x6d26c90) at python/sdist/amici/src/forwardproblem.cpp:203 #18 0x00007fffbc54fbf9 in amici::runAmiciSimulation (solver=..., edata=0x5078010, model=..., rethrow=false) at python/sdist/amici/src/amici.cpp:108 ``` --- include/amici/forwardproblem.h | 42 ++++++++++++++++++++++++---------- 1 file changed, 30 insertions(+), 12 deletions(-) diff --git a/include/amici/forwardproblem.h b/include/amici/forwardproblem.h index 40a4147f4e..cad434a3e1 100644 --- a/include/amici/forwardproblem.h +++ b/include/amici/forwardproblem.h @@ -441,19 +441,37 @@ class FinalStateStorer : public ContextManager { /** * @brief destructor, stores simulation state */ - ~FinalStateStorer() { + ~FinalStateStorer() noexcept(false) { if (fwd_) { - fwd_->final_state_ = fwd_->getSimulationState(); - // if there is an associated output timepoint, also store it in - // timepoint_states if it's not present there. - // this may happen if there is an error just at - // (or indistinguishably before) an output timepoint - auto final_time = fwd_->getFinalTime(); - auto const timepoints = fwd_->model->getTimepoints(); - if (!fwd_->timepoint_states_.count(final_time) - && std::find(timepoints.cbegin(), timepoints.cend(), final_time) - != timepoints.cend()) { - fwd_->timepoint_states_[final_time] = fwd_->final_state_; + try { + // This may throw in `CVodeSolver::getSens` + // due to https://github.com/LLNL/sundials/issues/82. + // Therefore, this dtor must be `noexcept(false)` to avoid + // programm termination. + fwd_->final_state_ = fwd_->getSimulationState(); + // if there is an associated output timepoint, also store it in + // timepoint_states if it's not present there. + // this may happen if there is an error just at + // (or indistinguishably before) an output timepoint + auto final_time = fwd_->getFinalTime(); + auto const timepoints = fwd_->model->getTimepoints(); + if (!fwd_->timepoint_states_.count(final_time) + && std::find(timepoints.cbegin(), timepoints.cend(), final_time) + != timepoints.cend()) { + fwd_->timepoint_states_[final_time] = fwd_->final_state_; + } + } catch (std::exception const&) { + // We must not throw in case we are already in the stack + // unwinding phase due to some other active exception, otherwise + // this will also lead to termination. + // + // In case there is another active exception, + // `fwd_->{final_state_,timepoint_states_}` won't be set, + // and we assume that they are either not accessed anymore, or + // that there is appropriate error handling in place. + if(!std::uncaught_exceptions()) { + throw; + } } } }