Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix segfaults from PvodeSolver #1926

Merged
merged 2 commits into from
Mar 2, 2020
Merged

Fix segfaults from PvodeSolver #1926

merged 2 commits into from
Mar 2, 2020

Conversation

johnomotani
Copy link
Contributor

@johnomotani johnomotani commented Feb 28, 2020

Solver::init() sets Solver::initialised=true. The destructor PvodeSolver::~PvodeSolver() uses initialised to check if it should free memory. Previously, Solver::init() was called near the start of PvodeSolver::init(), but this meant that Solver::initialised was set to true before the PvodeSolver had completed initialisation. An error during the initialisation (which is not unlikely, as CvodeMalloc calls the rhs function!) could then cause a segfault as the destructor free'd memory that had not been fully allocated. Calling Solver::init() at the end of PvodeSolver::init() ensures Solver::initialised is not set until PvodeSolver really is fully initialised.

This fixes the segfault @johnomotani reported in #1923.

@johnomotani
Copy link
Contributor Author

Not sure how this worked when I checked it before... Turns out Solver::init() needs to be called before Solver::getLocalN(), which is needed in PvodeSolver::init(). I think PvodeSolver needs a separate pvode_initialised flag.

Solver::init() sets Solver::initialised=true. The destructor
PvodeSolver::~PvodeSolver() previously used initialised to check if it
should free memory. Solver::init() is called near the start of
PvodeSolver::init(), so Solver::initialised is set to true before
PvodeSolver has completed initialisation. An error during the
initialisation (which is not unlikely, as CvodeMalloc calls the rhs
function!) could then cause a segfault as the destructor free'd memory
that had not been fully allocated. Using a separate pvode_initialised
flag, which is set to true only at the end of PvodeSolver::init() fixes
this issue.
CvodeSolver shares a similar structure, so potentially could have caused
a similar segfault.
@johnomotani johnomotani added the backport candidiate Does not break backward compatibility, so can be back-ported to v4.4 label Mar 1, 2020
@ZedThree ZedThree merged commit 77b228e into next Mar 2, 2020
@ZedThree ZedThree deleted the pvode-segfault-fix branch March 2, 2020 09:57
@ZedThree ZedThree mentioned this pull request Aug 4, 2021
28 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport candidiate Does not break backward compatibility, so can be back-ported to v4.4 bugfix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants