Modify

Opened 5 months ago

Closed 5 months ago

#17740 closed defect (fixed)

[PATCH] NPE in TagEditHelper

Reported by: taylor.smock Owned by: team
Priority: normal Milestone: 19.05
Component: Core Version:
Keywords: Cc:

Description

I don't know how to reproduce this bug, especially since TagEditHelper gets information from OsmDataManager.getInstance().getInProgressSelection() which uses DataSet.getSelected() which will ignore deleted primitives and (presumably) null primitives.

The only way I can think of to fix the NPE is to add a check somewhere to filter out primitives that are null, deleted, or have no DataSet. In this case, it looks like we where trying to call .getDataSet() on a null object.

URL:https://josm.openstreetmap.de/svn/trunk
Repository:UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b
Last:Changed Date: 2019-05-20 02:27:26 +0200 (Mon, 20 May 2019)
Build-Date:2019-05-20 01:31:08
Revision:15101
Relative:URL: ^/trunk

Identification: JOSM/1.5 (15101 en) Mac OS X 10.14.4
OS Build number: Mac OS X 10.14.4 (18E226)
Memory Usage: 1063 MB / 1820 MB (392 MB allocated, but free)
Java version: 1.8.0_211-b12, Oracle Corporation, Java HotSpot(TM) 64-Bit Server VM
Screen: Display 188947522 1920x1080, Display 69981408 2048x1152
Maximum Screen Size: 2048x1152
VM arguments: [-Djava.security.policy=file:<java.home>/lib/security/javaws.policy, -DtrustProxy=true, -Djnlpx.home=<java.home>/bin, -Djava.security.manager, -Djnlpx.origFilenameArg=${HOME}/Library/Application Support/Oracle/Java/Deployment/cache/6.0/31/583aa85f-7f8234e1, -Djnlpx.remove=false, -Dsun.awt.warmup=true, -Djava.util.Arrays.useLegacyMergeSort=true, -Djnlpx.heapsize=NULL,2048m, -Dmacosx.jnlpx.dock.name=JOSM (development version), -Dmacosx.jnlpx.dock.icon=${HOME}/Library/Application Support/Oracle/Java/Deployment/cache/6.0/25/4c122699-477b07e1.icns, -Djnlp.application.href=https://josm.openstreetmap.de/download/josm-latest.jnlp , -Djnlpx.jvm="<java.home>/bin/java"]
Dataset consistency test: No problems found

Plugins:
+ Mapillary (1.5.18)
+ apache-commons (34908)
+ apache-http (34908)
+ auto_tools (73)
+ buildings_tools (34982)
+ ejml (34908)
+ geotools (34908)
+ highwayNameModification (1557754861)
+ imagery_offset_db (34908)
+ jaxb (34908)
+ jna (34908)
+ jts (34908)
+ markseen (13)
+ opendata (34997)
+ osm-obj-info (51)
+ reltoolbox (34977)
+ turnrestrictions (34977)
+ utilsplugin2 (34977)

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

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

Last errors/warnings:
- W: java.io.IOException: Attribution is not loaded yet
- W: java.io.IOException: Attribution is not loaded yet
- W: java.io.IOException: Attribution is not loaded yet
- W: java.io.IOException: Attribution is not loaded yet
- W: java.io.IOException: Attribution is not loaded yet
- W: java.io.IOException: Attribution is not loaded yet
- W: java.io.IOException: Attribution is not loaded yet
- W: java.io.IOException: Attribution is not loaded yet
- E: Error header: The node with the id 1689682814 has already been deleted
- E: Handled by bug report queue: java.lang.NullPointerException


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

Warning issued by: BugReportExceptionHandler#handleException

=== STACK TRACE ===
Thread: AWT-EventQueue-2 (39) of javawsApplicationThreadGroup
java.lang.NullPointerException
	at org.openstreetmap.josm.command.ChangePropertyCommand.<init>(ChangePropertyCommand.java:108)
	at org.openstreetmap.josm.gui.dialogs.properties.TagEditHelper$EditTagDialog.performTagEdit(TagEditHelper.java:521)
	at org.openstreetmap.josm.gui.dialogs.properties.TagEditHelper.editTag(TagEditHelper.java:278)
	at org.openstreetmap.josm.gui.dialogs.properties.PropertiesDialog$EditAction.actionPerformed(PropertiesDialog.java:1127)
	at javax.swing.SwingUtilities.notifyAction(SwingUtilities.java:1668)
	at javax.swing.JComponent.processKeyBinding(JComponent.java:2882)
	at javax.swing.KeyboardManager.fireBinding(KeyboardManager.java:307)
	at javax.swing.KeyboardManager.fireKeyboardAction(KeyboardManager.java:263)
	at javax.swing.JComponent.processKeyBindingsForAllComponents(JComponent.java:2974)
	at javax.swing.JComponent.processKeyBindings(JComponent.java:2966)
	at javax.swing.JComponent.processKeyEvent(JComponent.java:2845)
	at java.awt.Component.processEvent(Component.java:6316)
	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.KeyboardFocusManager.redispatchEvent(KeyboardFocusManager.java:1954)
	at java.awt.DefaultKeyboardFocusManager.dispatchKeyEvent(DefaultKeyboardFocusManager.java:835)
	at java.awt.DefaultKeyboardFocusManager.preDispatchKeyEvent(DefaultKeyboardFocusManager.java:1103)
	at java.awt.DefaultKeyboardFocusManager.typeAheadAssertions(DefaultKeyboardFocusManager.java:974)
	at java.awt.DefaultKeyboardFocusManager.dispatchEvent(DefaultKeyboardFocusManager.java:800)
	at java.awt.Component.dispatchEventImpl(Component.java:4760)
	at java.awt.Container.dispatchEventImpl(Container.java:2297)
	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)

Attachments (2)

17740.patch (1.8 KB) - added by taylor.smock 5 months ago.
Modify the sel list such that there are no null objects, no deleted objects (duplicates some functionality), and no objects without a dataset.
17740_v2.patch (1.8 KB) - added by taylor.smock 5 months ago.
Lock the dataset before we get the selection to avoid disappearing primitives (apparently, finally always executes, so that is the only endUpdate() needed)

Download all attachments as: .zip

Change History (6)

Changed 5 months ago by taylor.smock

Attachment: 17740.patch added

Modify the sel list such that there are no null objects, no deleted objects (duplicates some functionality), and no objects without a dataset.

comment:1 Changed 5 months ago by Don-vip

We should rather try to detect the root cause instead of adding such workarounds.

comment:2 Changed 5 months ago by taylor.smock

It kind of looks like a concurrent modification issue, based off of the fact that we get the currently selected elements from a user POV, and somewhere between that and actually modifying the tags the node gets deleted.

I suppose we could issue a DataSet.beginUpdate() either before we get the the selection or once we get the selection(probably through OsmDataManager.getInstance().getActiveDataSet().beginUpdate();, but we would need to save the dataset for a finally block so we can run DataSet.endUpdate(), kind of like in 17740_v2.patch (to be attached).

Changed 5 months ago by taylor.smock

Attachment: 17740_v2.patch added

Lock the dataset before we get the selection to avoid disappearing primitives (apparently, finally always executes, so that is the only endUpdate() needed)

comment:3 Changed 5 months ago by Don-vip

Milestone: 19.05
Summary: [Possible PATCH] NPE in TagEditHelper[PATCH] NPE in TagEditHelper

comment:4 Changed 5 months ago by Don-vip

Resolution: fixed
Status: newclosed

In 15125/josm:

fix #17740 - Lock the dataset before we get the selection to avoid disappearing primitives (patch by taylor.smock)

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain team.
as The resolution will be set.
The resolution will be deleted.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.