Graphhopper fails to import file with missing tower node in road loop

I’m trying to import planet.pbf which we are keeping up to date with minute updates. This worked fine in old Graphopper 0.6, but now after update to 0.12 I can’t import the file anymore. Problem happens because one tower node which is part of a loop and a road is missing.

I can import this file fine with Graphopper 0.6 it seems the issue is loop detection in OSMReader#668 because bug happens when it tries to add node 5021809324 as the last node in Parking lot in addOSMWay. This fails in loop detection code because previous node 6749025851 is missing. lastGHNodeId is then set to -2 since it is a tower node (because it’s also part of a way). And inside handlePillarNode pillarInfo.getLatitude in for -5 (tmpNode -3)) throws exception.

If I delete tower nodes or pillar node from OSM data (but still keep them referenced in ways) import works. It seems issue is tower node part of a loop. Pillar node part of a loop works.

Simple file to cause the bug is in gist. It’s almost the same data as in OSM only node 6749025851 is deleted since it is missing in our data for some reason and highway=service tag is added to parking lot. Otherwise parking lot is skipped because of access rules and bug newer happens.

I used car encoder for testing.

Full stacktrace of master version of GH:
2019-09-04 12:58:57.591 [main] INFO com.graphhopper.reader.osm.OSMReader - 8, now parsing ways
2019-09-04 12:58:57.594 [main] ERROR com.graphhopper.reader.osm.OSMReader - Couldn’t properly add edge with osm ids:[5021809324, 5021809325, 5021809326, 5021809327, 6749025851, 5021809324]
java.lang.ArrayIndexOutOfBoundsException: -1
at com.graphhopper.storage.RAMDataAccess.getInt(RAMDataAccess.java:207)
at com.graphhopper.reader.PillarInfo.getLatitude(PillarInfo.java:83)
at com.graphhopper.reader.osm.OSMReader.handlePillarNode(OSMReader.java:784)
at com.graphhopper.reader.osm.OSMReader.addOSMWay(OSMReader.java:675)
at com.graphhopper.reader.osm.OSMReader.processWay(OSMReader.java:404)
at com.graphhopper.reader.osm.OSMReader.writeOsm2Graph(OSMReader.java:270)
at com.graphhopper.reader.osm.OSMReader.readGraph(OSMReader.java:146)
at com.graphhopper.GraphHopper.importData(GraphHopper.java:666)
at com.graphhopper.GraphHopper.process(GraphHopper.java:637)
at com.graphhopper.GraphHopper.importOrLoad(GraphHopper.java:615)
at com.graphhopper.http.GraphHopperManaged.start(GraphHopperManaged.java:74)
at com.graphhopper.http.cli.ImportCommand.run(ImportCommand.java:58)
at com.graphhopper.http.cli.ImportCommand.run(ImportCommand.java:39)
at io.dropwizard.cli.ConfiguredCommand.run(ConfiguredCommand.java:87)
at io.dropwizard.cli.Cli.run(Cli.java:78)
at io.dropwizard.Application.run(Application.java:93)
at com.graphhopper.http.GraphHopperApplication.main(GraphHopperApplication.java:34)
java.lang.RuntimeException: Couldn’t process file /home/markob/osm/ww_weirdnes/map_bug.osm, error: -1
at com.graphhopper.reader.osm.OSMReader.writeOsm2Graph(OSMReader.java:294)
at com.graphhopper.reader.osm.OSMReader.readGraph(OSMReader.java:146)
at com.graphhopper.GraphHopper.importData(GraphHopper.java:666)
at com.graphhopper.GraphHopper.process(GraphHopper.java:637)
at com.graphhopper.GraphHopper.importOrLoad(GraphHopper.java:615)
at com.graphhopper.http.GraphHopperManaged.start(GraphHopperManaged.java:74)
at com.graphhopper.http.cli.ImportCommand.run(ImportCommand.java:58)
at com.graphhopper.http.cli.ImportCommand.run(ImportCommand.java:39)
at io.dropwizard.cli.ConfiguredCommand.run(ConfiguredCommand.java:87)
at io.dropwizard.cli.Cli.run(Cli.java:78)
at io.dropwizard.Application.run(Application.java:93)
at com.graphhopper.http.GraphHopperApplication.main(GraphHopperApplication.java:34)
Caused by: java.lang.ArrayIndexOutOfBoundsException: -1
at com.graphhopper.storage.RAMDataAccess.getInt(RAMDataAccess.java:207)
at com.graphhopper.reader.PillarInfo.getLatitude(PillarInfo.java:83)
at com.graphhopper.reader.osm.OSMReader.handlePillarNode(OSMReader.java:784)
at com.graphhopper.reader.osm.OSMReader.addOSMWay(OSMReader.java:675)
at com.graphhopper.reader.osm.OSMReader.processWay(OSMReader.java:404)
at com.graphhopper.reader.osm.OSMReader.writeOsm2Graph(OSMReader.java:270)
… 11 more

It’s almost the same data as in OSM only node 6749025851 is deleted since it is missing in our data for some reason and highway=service tag is added to parking lot.

What do you mean here as I can see the node 6749025851 in the gist?

When I try to import the osm file gist I can reproduce the error and it looks like a valid constellation. So this should not happen and is likely a bug. We would appreciate a fix if you have time to provide a pull request?

Node 6749025851 is still referenced. But there is no node information since node tag is missing.
I can try to provide a fix but I’m not so at home in GH codebase. What should be expected result? Since in this case if loop is created it wouldn’t be added to the graph since tower node which is connecting loop and way is missing. So loop wouldn’t be connected to the rest of the graph.

And which function is actually wrong? I would assume that handlePillarNode shouldn’t be called when lastGHNodeId is -2 since it seems strange that handlePillarNode should be called with tower node. But handlePillarNode is also called when pillar node has to be converted to towerNode.

Do you have any other hint?

And which function is actually wrong?

I have no idea yet. Maybe something of the loop removal is wrong? https://github.com/graphhopper/graphhopper/pull/1531/files#diff-9dd148fc7375c78c2591b56b2cbe5cc8L606