[Patch] IAE: "Node pair not part of way" when splitting way

[jorge@…]

What steps will reproduce the problem?

What is the expected result?

What happens instead?

Please provide any additional information below. Attach a screenshot if possible.

URL:https://josm.openstreetmap.de/svn/trunk
Repository:UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b
Last:Changed Date: 2021-04-27 20:35:33 +0200 (Tue, 27 Apr 2021)
Build-Date:2021-04-27 21:58:39
Revision:17833
Relative:URL: ^/trunk

Identification: JOSM/1.5 (17833 en) Mac OS X 10.16
OS Build number: macOS 11.4 (20F71)
Memory Usage: 1905 MB / 1905 MB (846 MB allocated, but free)
Java version: 1.8.0_291-b10, Oracle Corporation, Java HotSpot(TM) 64-Bit Server VM
Look and Feel: com.apple.laf.AquaLookAndFeel
Screen: Display 69732992 1280×800 (scaling 1.00×1.00) Display 188956746 1920×1080 (scaling 1.00×1.00) Display 188956681 1920×1080 (scaling 1.00×1.00)
Maximum Screen Size: 1920×1080
Best cursor sizes: 16×16→16×16, 32×32→32×32
Environment variable LANG: en_US.UTF-8
System property file.encoding: UTF-8
System property sun.jnu.encoding: UTF-8
Locale info: en_US
Numbers with default locale: 1234567890 -> 1234567890
Dataset consistency test: No problems found

Plugins:
+ Mapillary (1.5.37.6)
+ apache-commons (35524)
+ apache-http (35589)
+ auto_tools (73)
+ buildings_tools (35740)
+ changeset-viewer (25)
+ easypresets (1616854084)
+ graphview (35640)
+ intersection (0.0.7)
+ jna (35662)
+ markseen (14)
+ openqa (0.2.2)
+ osm-obj-info (56)
+ rex (53)
+ tageditor (35640)
+ turnlanes (35640)
+ turnrestrictions (35640)
+ utilsplugin2 (35691)

Map paint styles:
+ https://josm.openstreetmap.de/josmfile?page=Styles/Lane_and_Road_Attributes&zip=1
+ https://josm.openstreetmap.de/josmfile?page=Styles/Coloured_Streets&zip=1
+ https://raw.githubusercontent.com/KaartGroup/Kaart-Styles/master/Overlapping%20Ways.mapcss 
+ https://raw.githubusercontent.com/KaartGroup/Kaart-Styles/master/Kaart-Styles.mapcss 

Validator rules:
+  https://raw.githubusercontent.com/KaartGroup/KaartValidator/master/kaart.clingstone.validator.mapcss
+ https://raw.githubusercontent.com/KaartGroup/KaartValidator/master/kaart.clingstone.validator.mapcss 

Last errors/warnings:
- 04047.094 E: Unknown detection "human--person--individual"
- 04047.097 E: Unknown detection "human--person--individual"
- 04051.898 E: Unknown detection "human--person--individual"
- 04051.899 E: Unknown detection "human--person--individual"
- 04055.719 E: Unknown detection "human--person--individual"
- 04065.031 E: Unknown detection "human--person--individual"
- 04071.801 E: Unknown detection "object--traffic-sign"
- 04071.802 E: Unknown detection "object--traffic-sign"
- 04088.782 E: Handled by bug report queue: java.lang.IllegalArgumentException: Node pair is not part of way!
- 04088.791 E: Handled by bug report queue: java.lang.IllegalArgumentException: Node pair is not part of way!



=== REPORTED CRASH DATA ===
BugReportExceptionHandler#handleException:
No data collected.

Warning issued by: BugReportExceptionHandler#handleException

=== STACK TRACE ===
Thread: AWT-EventQueue-0 (19) of main
java.lang.IllegalArgumentException: Node pair is not part of way!
	at org.openstreetmap.josm.data.osm.WaySegment.forNodePair(WaySegment.java:70)
	at org.openstreetmap.josm.actions.SplitWayAction$SegmentToKeepSelectionDialog.lambda$configureList$0(SplitWayAction.java:202)
	at javax.swing.JList.fireSelectionValueChanged(JList.java:1802)
	at javax.swing.JList$ListSelectionHandler.valueChanged(JList.java:1816)
	at javax.swing.DefaultListSelectionModel.fireValueChanged(DefaultListSelectionModel.java:184)
	at javax.swing.DefaultListSelectionModel.fireValueChanged(DefaultListSelectionModel.java:164)
	at javax.swing.DefaultListSelectionModel.fireValueChanged(DefaultListSelectionModel.java:211)
	at javax.swing.DefaultListSelectionModel.changeSelection(DefaultListSelectionModel.java:405)
	at javax.swing.DefaultListSelectionModel.changeSelection(DefaultListSelectionModel.java:415)
	at javax.swing.DefaultListSelectionModel.setSelectionInterval(DefaultListSelectionModel.java:459)
	at javax.swing.JList.setSelectionInterval(JList.java:2071)
	at javax.swing.plaf.basic.BasicListUI$Handler.adjustSelection(BasicListUI.java:2739)
	at javax.swing.plaf.basic.BasicListUI$Handler.mousePressed(BasicListUI.java:2695)
	at javax.swing.plaf.basic.BasicListUI$MouseInputHandler.mousePressed(BasicListUI.java:1514)
	at java.awt.AWTEventMulticaster.mousePressed(AWTEventMulticaster.java:280)
	at java.awt.Component.processMouseEvent(Component.java:6536)
	at javax.swing.JComponent.processMouseEvent(JComponent.java:3324)
	at java.awt.Component.processEvent(Component.java:6304)
	at java.awt.Container.processEvent(Container.java:2239)
	at java.awt.Component.dispatchEventImpl(Component.java:4889)
	at java.awt.Container.dispatchEventImpl(Container.java:2297)
	at java.awt.Component.dispatchEvent(Component.java:4711)
	at java.awt.LightweightDispatcher.retargetMouseEvent(Container.java:4904)
	at java.awt.LightweightDispatcher.processMouseEvent(Container.java:4532)
	at java.awt.LightweightDispatcher.dispatchEvent(Container.java:4476)
	at java.awt.Container.dispatchEventImpl(Container.java:2283)
	at java.awt.Window.dispatchEventImpl(Window.java:2746)
	at java.awt.Component.dispatchEvent(Component.java:4711)
	at java.awt.EventQueue.dispatchEventImpl(EventQueue.java:760)
	at java.awt.EventQueue.access$500(EventQueue.java:97)
	at java.awt.EventQueue$3.run(EventQueue.java:709)
	at java.awt.EventQueue$3.run(EventQueue.java:703)
	at java.security.AccessController.doPrivileged(Native Method)
	at java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:74)
	at java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:84)
	at java.awt.EventQueue$4.run(EventQueue.java:733)
	at java.awt.EventQueue$4.run(EventQueue.java:731)
	at java.security.AccessController.doPrivileged(Native Method)
	at java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:74)
	at java.awt.EventQueue.dispatchEvent(EventQueue.java:730)
	at java.awt.EventDispatchThread.pumpOneEventForFilters(EventDispatchThread.java:205)
	at java.awt.EventDispatchThread.pumpEventsForFilter(EventDispatchThread.java:116)
	at java.awt.EventDispatchThread.pumpEventsForHierarchy(EventDispatchThread.java:105)
	at java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:101)
	at java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:93)
	at java.awt.EventDispatchThread.run(EventDispatchThread.java:82)

[Bjoeni]

Appears to happen when the selected way is removed while the SegmentToKeepSelectionDialog (dialog asking which segment should keep the history of the way) is still open.

[Bjoeni]

Also happened when adding or removing nodes from the way

[Bjoeni]

Ticket #20993 has been marked as a duplicate of this ticket.

[Don-vip]

In 17999/josm:

fix #20935 - fix IAE: "Node pair not part of way" when splitting way (patch by Bjoeni)

[Don-vip]

In 18004/josm:

fix #21102 - see #20935 - revert r17999

[Don-vip]

The patch makes it worse and caused a major regression. I'm reverting it and releasing an hotfix.

[skyper]

Replying to Bjoeni:

Appears to happen when the selected way is removed while the SegmentToKeepSelectionDialog (dialog asking which segment should keep the history of the way) is still open.

Shouldn't this dialog block all further actions?

[Bjoeni]

Replying to skyper:

Replying to Bjoeni:

Appears to happen when the selected way is removed while the SegmentToKeepSelectionDialog (dialog asking which segment should keep the history of the way) is still open.

Shouldn't this dialog block all further actions?

Someone before me decided to use a non-modal dialog. I think the reason is that when the selected way segment is highlighted the user should still be able to pan and zoom.
Also, whilst it would have avoided all these problems I think it shouldn't really be an issue once properly implemented. So I'd leave that as is.

---

Replying to Don-vip:

The patch makes it worse and caused a major regression. I'm reverting it and releasing an hotfix.

According to the stack trace of #21101 it happened at least that time as a result of an undo action. The only way I could reproduce it was when a selected node is removed while the dialog is opened. But that causes an exception without this patch too.

@Vincent, you said that it happened for you when the dialog is not visible? Does that mean expert mode disabled or what do you mean?

Either way, fixing it is easy (just an additional null check, and closing the dialog when buildSplitChunks() returns null). But I'd like to understand what went wrong first.

[Bjoeni]

This should fix it. But I'll mark it WIP until we can reproduce it somehow.

[Don-vip]

in #21101 I was able to reproduce it at the second split on a different way, after I chose the "remember my choice" option in the dialog displayed for the split of the first way (which ran OK). But the OP in #21101 says nothing about that.

[skyper]

Replying to Bjoeni:

Replying to skyper:

Shouldn't this dialog block all further actions?

Someone before me decided to use a non-modal dialog. I think the reason is that when the selected way segment is highlighted the user should still be able to pan and zoom.

Yes, that is the reason.

Also, whilst it would have avoided all these problems I think it shouldn't really be an issue once properly implemented.

Well, I can think of:

• reverting the way direction
• removing the split node from the dataset or the way
  • merging nodes can lead to this, too
• splitting the way with another action

Do not know if that all is that easy.
What happens if you update data in between and the split node is removed as it was deleted by another user, mean-while? Is the dialog updated to reflect these changes?

[Bjoeni]

Replying to skyper:

Replying to Bjoeni:

Replying to skyper:

Shouldn't this dialog block all further actions?

Someone before me decided to use a non-modal dialog. I think the reason is that when the selected way segment is highlighted the user should still be able to pan and zoom.

Yes, that is the reason.

Also, whilst it would have avoided all these problems I think it shouldn't really be an issue once properly implemented.

Well, I can think of:

• reverting the way direction

Doesn't cause any issues with the patch above. Only if you split it in more than two parts and select one end, the other end will be selected in the dialog. So if you have 5 nodes - 10 nodes - 15 nodes and revert it, the dialog then shows 15 nodes - 10 nodes - 5 nodes. If you had the first item selected before reverting, it'll mean that the first item stays selected and the segment with 15 nodes is then selected to keep the history. Which I think isn't really an issue.

• removing the split node from the dataset or the way
  • merging nodes can lead to this, too

not an issue, the dialog is updated

• splitting the way with another action

not possible

Do not know if that all is that easy.
What happens if you update data in between and the split node is removed as it was deleted by another user, mean-while? Is the dialog updated to reflect these changes?

I did not specifically test this, but I assume that this fires the usual DataSetListener events and is not any different from the user manually adding / removing a node.

That was actually the whole point of this patch, as currently all the examples you mentioned
a) either fail with the IAE "Node pair not part of way" or
b) potentially even worse leave individual nodes somewhere that are not part of the way anymore.

[Bjoeni]

Replying to Don-vip:

in #21101 I was able to reproduce it at the second split on a different way, after I chose the "remember my choice" option in the dialog displayed for the split of the first way (which ran OK). But the OP in #21101 says nothing about that.

Ok, in that case for me it doesn't crash anymore. Part of the problem was that the DataSetListener was added but not removed when the dialog wasn't shown in the first place (cause then setVisible(false) was never called). Also I added an additional null check in case the selected node is removed while the dialog is opened. Both should fixed in v2 above, could you please have a look if it's still reproducible for you?

[Don-vip]

Seems fine now!

[Don-vip]

In 18024/josm:

fix #20935 - fix IAE: "Node pair not part of way" when splitting way (second patch by Bjoeni)