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.

Sign up for GitHub

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

Jump to bottom

Propagate exceptions across threads #417

Closed
jcoupey opened this issue Dec 15, 2020 · 1 comment · Fixed by #479
Closed

Propagate exceptions across threads #417

jcoupey opened this issue Dec 15, 2020 · 1 comment · Fixed by #479
Labels
bug good first issue internals
Milestone
v1.10.0

Comments

@jcoupey
Copy link
Collaborator

jcoupey commented Dec 15, 2020

We usually run parallel solving across multiple threads, e.g. for CVRP here. The problem is that if a worker thread were to throw for an unexpected reason, the current code would not propagate the exception to the main thread. I simulated this by throwing a std::runtime_error from LocalSearch::run, which result in an ugly:

terminate called recursively
terminate called after throwing an instance of 'Aborted (core dumped)

whereas we'd like to gracefully exit with what feedback we can {"code": 1, "Internal error: ..."}.

The good news is I've had no report of this kind of failure so far but we should nevertheless use std::exception_ptr to handle this correctly, see e.g. https://vorbrodt.blog/2019/03/24/propagate-exceptions-across-threads/.
@jcoupey jcoupey added this to the v1.10.0 milestone Feb 7, 2021
@jcoupey
Copy link
Collaborator Author

jcoupey commented Mar 29, 2021

The good news is I've had no report of this kind of failure so far

Obviously this is related to the fact that the lambda used with std::thread only calls functions that are free of any throw clause (heuristics and local search stuff). So basically we're safe as far as our own code is concerned, but not if some standard library call does throw somewhere down the line.

My first though was that we should mark the lambda function (and maybe functions used by it) as noexcept. While this may be a good practice, it does not guarantee a compile-time check ensuring no exception will ever go through. The behavior when marking the lamba as noexcept and throwing from inside is that we get a call to std::terminate. This does not solve the need for a clean exit with an error.

My conclusion is that we should still catch/rethrow exceptions from the threads to be safe. Happy to have some feedback from others with more experience though!

This was referenced Mar 29, 2021
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug good first issue internals
Projects
None yet
Milestone
v1.10.0
Development

Successfully merging a pull request may close this issue.

Handle exceptions across threads VROOM-Project/vroom
1 participant
@jcoupey