Bugfix: Prevent GetShortcut to run into an infinite loop. #4532
Conversation
|
any chance we can add a gurka test for this? you seem to have quite a well described case here. of course the test would run for ever but after your fix the test should just exit normally 😄 would you be up for trying to add a test to here: https://github.com/valhalla/valhalla/blob/master/test/gurka/test_shortcut.cc |
test/gurka/test_shortcut.cc
Outdated
| )"; | ||
| const gurka::ways ways = { | ||
| {"AB", | ||
| {{"highway", "primary"}}}, |
There was a problem hiding this comment.
The test is simple and effective, thanks. However, I don’t think this would fail without your PR. I think the edges need more common attributes for them to form a shortcut IIRC, e.g. the same name.
There was a problem hiding this comment.
I've tried and it runs into infinite loop without my PR, because GetShortcut try to find a shortcut and run along the edges in the loop. It doesn't fail, but run for ever without my PR.
There was a problem hiding this comment.
Ok perfect, I was pretty sure I was pulling my hair out a few times when it didn't produce shortcuts for some reason when I didn't include any other properties like name or so. Thanks for the feedback, LGTM.
|
@lakovacs2 if you can pull master into your branch we can merge |
Issue
If there is a loop of the edges that are all on the same level, and all the connecting edges to the loop are on other levels, and there are no shortcuts in the loop, then the original GraphReader::GetShortcut runs into infinite loop.
E.g. if there is a roundabout on level 1 and all the connecting edges are on level 2.
Lajos Kovács lajos.kovacs@nng.com NNG LLC.