Modify

Opened 12 years ago

Closed 12 years ago

#7979 closed defect (fixed)

ArrayIndexOutOfBoundsException when rendering ways

Reported by: anonymous Owned by: team
Priority: major Milestone:
Component: Core Version: latest
Keywords: Cc: mcheck, richlv, naoliv

Description (last modified by skyper)

Repository Root: http://josm.openstreetmap.de/svn
Build-Date: 2012-08-18 01:31:08
Last Changed Author: jttt
Revision: 5449
Repository UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b
URL: http://josm.openstreetmap.de/svn/trunk
Last Changed Date: 2012-08-17 07:45:35 +0200 (Fri, 17 Aug 2012)
Last Changed Rev: 5449

Identification: JOSM/1.5 (5449 it)
Memory Usage: 186 MB / 455 MB (58 MB allocated, but free)
Java version: 1.6.0_24, Sun Microsystems Inc., OpenJDK Server VM
Operating system: Linux
Dataset consistency test: No problems found

Plugin: FixAddresses (28412)
Plugin: ImageWayPoint (27852)
Plugin: ImportImagePlugin (28412)
Plugin: SimplifyArea (27905)
Plugin: buildings_tools (28529)
Plugin: continuosDownload (28375)
Plugin: contourmerge (1003)
Plugin: imageryadjust (28412)
Plugin: irsrectify (28412)
Plugin: mirrored_download (28418)
Plugin: multipoly-convert (28412)
Plugin: namemanager (28412)
Plugin: openstreetbugs (28412)
Plugin: osmarender (28412)
Plugin: plastic_laf (26605)
Plugin: reverter (28535)
Plugin: tageditor (28412)
Plugin: tagging-preset-tester (28412)
Plugin: terracer (28412)

java.lang.ArrayIndexOutOfBoundsException: 14
	at org.openstreetmap.josm.data.osm.Way.getNode(Way.java:116)
	at org.openstreetmap.josm.data.osm.WaySegment.getSecondNode(WaySegment.java:29)
	at org.openstreetmap.josm.data.osm.WaySegment.toWay(WaySegment.java:39)
	at org.openstreetmap.josm.data.osm.visitor.paint.MapPainter.drawVirtualNodes(MapPainter.java:1169)
	at org.openstreetmap.josm.data.osm.visitor.paint.StyledMapRenderer.render(StyledMapRenderer.java:234)
	at org.openstreetmap.josm.gui.layer.OsmDataLayer.paint(OsmDataLayer.java:281)
	at org.openstreetmap.josm.gui.MapView.paintLayer(MapView.java:495)
	at org.openstreetmap.josm.gui.MapView.paint(MapView.java:579)
	at javax.swing.JComponent.paintChildren(JComponent.java:866)
	at javax.swing.JComponent.paint(JComponent.java:1038)
	at javax.swing.JComponent.paintToOffscreen(JComponent.java:5138)
	at javax.swing.RepaintManager$PaintManager.paintDoubleBuffered(RepaintManager.java:1454)
	at javax.swing.RepaintManager$PaintManager.paint(RepaintManager.java:1385)
	at javax.swing.BufferStrategyPaintManager.paint(BufferStrategyPaintManager.java:318)
	at javax.swing.RepaintManager.paint(RepaintManager.java:1188)
	at javax.swing.JComponent._paintImmediately(JComponent.java:5086)
	at javax.swing.JComponent.paintImmediately(JComponent.java:4896)
	at javax.swing.RepaintManager.paintDirtyRegions(RepaintManager.java:783)
	at javax.swing.RepaintManager.paintDirtyRegions(RepaintManager.java:735)
	at javax.swing.RepaintManager.prePaintDirtyRegions(RepaintManager.java:677)
	at javax.swing.RepaintManager.access$700(RepaintManager.java:58)
	at javax.swing.RepaintManager$ProcessingRunnable.run(RepaintManager.java:1593)
	at java.awt.event.InvocationEvent.dispatch(InvocationEvent.java:226)
	at java.awt.EventQueue.dispatchEventImpl(EventQueue.java:647)
	at java.awt.EventQueue.access$000(EventQueue.java:96)
	at java.awt.EventQueue$1.run(EventQueue.java:608)
	at java.awt.EventQueue$1.run(EventQueue.java:606)
	at java.security.AccessController.doPrivileged(Native Method)
	at java.security.AccessControlContext$1.doIntersectionPrivilege(AccessControlContext.java:105)
	at java.awt.EventQueue.dispatchEvent(EventQueue.java:617)
	at java.awt.EventDispatchThread.pumpOneEventForFilters(EventDispatchThread.java:275)
	at java.awt.EventDispatchThread.pumpEventsForFilter(EventDispatchThread.java:200)
	at java.awt.EventDispatchThread.pumpEventsForHierarchy(EventDispatchThread.java:190)
	at java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:185)
	at java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:177)
	at java.awt.EventDispatchThread.run(EventDispatchThread.java:138)

Attachments (1)

7979.patch (132.9 KB ) - added by Don-vip 12 years ago.

Download all attachments as: .zip

Change History (32)

comment:1 by skyper, 12 years ago

Description: modified (diff)
Version: latest

Please, if possible, submit a step by step describtion how to reproduce this error. At least tell us your last actions before this error occured.

Thanks

Last edited 12 years ago by skyper (previous) (diff)

comment:2 by mdk, 12 years ago

I look into the code and it seems to be very simple:

public Node getSecondNode(){

return way.getNode(lowerIndex + 1);

}

will fail on ways with only one node.

in reply to:  2 comment:3 by skyper, 12 years ago

Replying to mdk:

will fail on ways with only one node.

A way always needs at least two nodes, therefor a way with only one node should not exist at all. Anyway patches are always welcome.

in reply to:  description comment:4 by bastiK, 12 years ago

Replying to anonymous:

java.lang.ArrayIndexOutOfBoundsException: 14
                                          ^^

This suggests, that the way had more than one node.

comment:5 by bastiK, 12 years ago

It looks like a race condition, but DataSet should be Thread save in principle.

comment:6 by stoecker, 12 years ago

Resolution: irreproducible
Status: newclosed

Further information when happening again would be fine.

comment:7 by stoecker, 12 years ago

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

comment:8 by stoecker, 12 years ago

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

comment:9 by bastiK, 12 years ago

It is indeed unsafe:

  • the DataSet.highlightedVirtualNodes field is set from SelectAction when user moves mouse
  • It is used during painting. In the meantime the way might have changed, e.g. deleted with a shortcut.

in reply to:  9 comment:10 by Don-vip, 12 years ago

Replying to bastiK:

It is indeed unsafe:

  • the DataSet.highlightedVirtualNodes field is set from SelectAction when user moves mouse
  • It is used during painting. In the meantime the way might have changed, e.g. deleted with a shortcut.

I think it's that, I just got it for the first time:

Repository Root: http://josm.openstreetmap.de/svn
Build-Date: 2012-09-06 01:31:06
Last Changed Author: bastiK
Revision: 5501
Repository UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b
URL: http://josm.openstreetmap.de/svn/trunk
Last Changed Date: 2012-09-05 22:15:03 +0200 (Wed, 05 Sep 2012)
Last Changed Rev: 5501

Identification: JOSM/1.5 (5501 en)
Memory Usage: 233 MB / 247 MB (87 MB allocated, but free)
Java version: 1.7.0_07, Oracle Corporation, Java HotSpot(TM) Client VM
Operating system: Windows 7
Dataset consistency test: No problems found

Plugin: cadastre-fr (28656)
Plugin: openstreetbugs (28656)
Plugin: turnrestrictions (28656)

java.lang.ArrayIndexOutOfBoundsException: 22
	at org.openstreetmap.josm.data.osm.Way.getNode(Way.java:116)
	at org.openstreetmap.josm.data.osm.WaySegment.getSecondNode(WaySegment.java:29)
	at org.openstreetmap.josm.data.osm.WaySegment.toWay(WaySegment.java:39)
	at org.openstreetmap.josm.data.osm.visitor.paint.MapPainter.drawVirtualNodes(MapPainter.java:1169)
	at org.openstreetmap.josm.data.osm.visitor.paint.StyledMapRenderer.render(StyledMapRenderer.java:234)
	at org.openstreetmap.josm.gui.layer.OsmDataLayer.paint(OsmDataLayer.java:283)
	at org.openstreetmap.josm.gui.MapView.paintLayer(MapView.java:488)
	at org.openstreetmap.josm.gui.MapView.paint(MapView.java:572)
	at javax.swing.JComponent.paintChildren(Unknown Source)
	at javax.swing.JComponent.paint(Unknown Source)
	at javax.swing.JComponent.paintChildren(Unknown Source)
	at javax.swing.JSplitPane.paintChildren(Unknown Source)
	at javax.swing.JComponent.paint(Unknown Source)
	at javax.swing.JComponent.paintChildren(Unknown Source)
	at javax.swing.JComponent.paint(Unknown Source)
	at javax.swing.JComponent.paintToOffscreen(Unknown Source)
	at javax.swing.RepaintManager$PaintManager.paintDoubleBuffered(Unknown Source)
	at javax.swing.RepaintManager$PaintManager.paint(Unknown Source)
	at javax.swing.RepaintManager.paint(Unknown Source)
	at javax.swing.JComponent._paintImmediately(Unknown Source)
	at javax.swing.JComponent.paintImmediately(Unknown Source)
	at javax.swing.RepaintManager.paintDirtyRegions(Unknown Source)
	at javax.swing.RepaintManager.paintDirtyRegions(Unknown Source)
	at javax.swing.RepaintManager.prePaintDirtyRegions(Unknown Source)
	at javax.swing.RepaintManager.access$700(Unknown Source)
	at javax.swing.RepaintManager$ProcessingRunnable.run(Unknown Source)
	at java.awt.event.InvocationEvent.dispatch(Unknown Source)
	at java.awt.EventQueue.dispatchEventImpl(Unknown Source)
	at java.awt.EventQueue.access$200(Unknown Source)
	at java.awt.EventQueue$3.run(Unknown Source)
	at java.awt.EventQueue$3.run(Unknown Source)
	at java.security.AccessController.doPrivileged(Native Method)
	at java.security.ProtectionDomain$1.doIntersectionPrivilege(Unknown Source)
	at java.awt.EventQueue.dispatchEvent(Unknown Source)
	at java.awt.EventDispatchThread.pumpOneEventForFilters(Unknown Source)
	at java.awt.EventDispatchThread.pumpEventsForFilter(Unknown Source)
	at java.awt.EventDispatchThread.pumpEventsForHierarchy(Unknown Source)
	at java.awt.EventDispatchThread.pumpEvents(Unknown Source)
	at java.awt.EventDispatchThread.pumpEvents(Unknown Source)
	at java.awt.EventDispatchThread.run(Unknown Source)

I deleted a node from a building with Del key and at the same time moved the mouse over another node of the same way -> Exception.

comment:11 by Don-vip, 12 years ago

Resolution: irreproducible
Status: closedreopened

comment:12 by Don-vip, 12 years ago

Summary: josm latest bug reportArrayIndexOutOfBoundsException when rendering ways

comment:13 by bastiK, 12 years ago

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

comment:14 by Don-vip, 12 years ago

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

comment:15 by skyper, 12 years ago

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

comment:16 by anonymous, 12 years ago

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

comment:17 by skyper, 12 years ago

Cc: mcheck added
Priority: normalmajor

comment:18 by skyper, 12 years ago

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

comment:19 by Don-vip, 12 years ago

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

comment:20 by Don-vip, 12 years ago

Cc: richlv added

comment:21 by Don-vip, 12 years ago

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

comment:22 by Don-vip, 12 years ago

Cc: naoliv added

comment:23 by Don-vip, 12 years ago

We need to fix this bug for the next release :)

comment:24 by bastiK, 12 years ago

I agree, any ideas?

in reply to:  24 comment:25 by Don-vip, 12 years ago

Replying to bastiK:

I agree, any ideas?

Not yet, I was hoping you had one :D

comment:26 by Don-vip, 12 years ago

First idea: delay the call to data.getHighlightedVirtualNodes() in StyledMapRenderer.render() and WireframeMapRenderer.render().

If I call this method just before the for() loop, I gain between 1.2 and 1.6 ms. It does not "solve" the bug but should drastically reduce its appearance probability ?

comment:27 by bastiK, 12 years ago

Hm, Ok... :)

What about this?

try {
 ...
} catch (Exception e) {}

comment:28 by Don-vip, 12 years ago

We can combine the two ideas :)

by Don-vip, 12 years ago

Attachment: 7979.patch added

comment:29 by Don-vip, 12 years ago

Here's a big patch with these two ideas, plus a lot of refactoring. While fixing this bug, I noticed a lot of common code between MapPainter and WireframeMapRenderer.

To get all this code in a single class (AbstractMapRenderer), I merged MapPainter and StyledMapRenderer.

Let me know if it's ok to integrate the refactoring in this release or if you prefer to wait for the next development cycle.

in reply to:  29 comment:30 by bastiK, 12 years ago

Replying to Don-vip:

Here's a big patch with these two ideas, plus a lot of refactoring. While fixing this bug, I noticed a lot of common code between MapPainter and WireframeMapRenderer.

To get all this code in a single class (AbstractMapRenderer), I merged MapPainter and StyledMapRenderer.

An alternative would be to add new classes AbstractMapPainter and WireframeMapPainter, but I think I prefer the merging.

Let me know if it's ok to integrate the refactoring in this release or if you prefer to wait for the next development cycle.

Not in the spirit of bug-fixing, but should be relatively harmless as far as I can see.

--

I guess it's not the best style to use Exception handling to get your array indices right (my idea, I know :) ). The only disadvantage seems to be, that in this part of the code, ArrayIndexOutOfBoundsExceptions will silently get caught even if they are not caused by the same issue. This seems acceptable.

Another approach would be to avoid the WaySegment class and add another data class with { Way way; Node firstNode, secondNode; int firstIdx; }. On read, it would first check, if the way still has firstIdx+2 nodes, then check if the corresponding way nodes are still the same and report changes in a more controlled manner.

comment:31 by Don-vip, 12 years ago

Resolution: fixed
Status: reopenedclosed

In 5571/josm:

fix #7979 - fix ArrayIndexOutOfBoundsException when rendering ways with the "quick and dirty" approach, and a TODO note to code a better approach on the next development cycle

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. Next status will be 'reopened'.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.