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

Temporary fix for trace_route crash #2322

Merged
merged 3 commits into from
Apr 17, 2020
Merged

Conversation

dnesbitt61
Copy link
Member

Check match_result_itr after it is incremented to make sure it doesn't iterate past the end of match_results. Note - this is just a quick fix to throw an exception rather than causing a crash. There may be more graceful ways to continue forming the path.

fixes #2321

Tasklist

  • Add tests
  • Add #fixes with the issue number that this PR addresses
  • Generally use squash merge to rebase and clean comments before merging
  • Update the changelog

Requirements / Relations

Link any requirements here. Other pull requests this PR is based on?

@@ -446,6 +446,11 @@ thor_worker_t::map_match(Api& request) {
// (NOTE that, in between edges may have same edge id with the last edge) right before where
// discontinuity occurs
auto prev_match_result = match_result_itr++;
if (match_result_itr >= match_results.cend()) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we are in the middle of refactoring this, so i think this is good enough for now until the refactor lands (which will delete all this extremely error-prone code)

@dnesbitt61 dnesbitt61 merged commit 9ae5ba9 into master Apr 17, 2020
@dnesbitt61 dnesbitt61 deleted the trace_route_crash_fix branch April 17, 2020 12:00
yuzheyan added a commit that referenced this pull request Apr 20, 2020
… into refactor_merge_segment

* 'refactor_merge_segment' of github.com:valhalla/valhalla:
  fix shape trimming for map matching (gurka/actor refactor + macos ccache) (#2326)
  Temporary fix for trace_route crash (#2322)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

trace_route crash
2 participants