Modify

Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#12598 closed defect (fixed)

ConcurrentModificationException in StyledMapRenderer.render after validation and moving map view

Reported by: SanderH Owned by: team
Priority: normal Milestone: 16.04
Component: Core mappaint Version:
Keywords: template_report fork join multi thread exception Cc: simon04, bastiK, michael2402

Description

What steps will reproduce the problem?

Unsure how to replicate.
I had just ran the validator (after resolving many errors after previous validator checks) and was just scrolling the map view when the error popped up.

What is the expected result?

What happens instead?

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

URL:http://josm.openstreetmap.de/svn/trunk
Repository:UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b
Last:Changed Date: 2016-02-28 14:07:49 +0100 (Sun, 28 Feb 2016)
Build-Date:2016-02-28 22:44:23
Revision:9900
Relative:URL: ^/trunk

Identification: JOSM/1.5 (9900 en) Windows 10 64-Bit
Memory Usage: 1190 MB / 1806 MB (638 MB allocated, but free)
Java version: 1.8.0_74-b02, Oracle Corporation, Java HotSpot(TM) 64-Bit Server VM
VM arguments: [-Djava.security.manager, -Djava.security.policy=file:C:\Program Files\Java\jre1.8.0_74\lib\security\javaws.policy, -DtrustProxy=true, -Djnlpx.home=<java.home>\bin, -Djnlpx.origFilenameArg=C:\Users\Sander\AppData\LocalLow\Sun\Java\Deployment\cache\6.0\56\1ee8cfb8-5ee29e1d, -Djnlpx.remove=false, -Djava.util.Arrays.useLegacyMergeSort=true, -Djnlpx.splashport=15702, -Djnlp.application.href=https://josm.openstreetmap.de/download/josm.jnlp, -Djnlpx.jvm=<java.home>\bin\javaw.exe, -Djnlpx.vmargs=LURqYXZhLnV0aWwuQXJyYXlzLnVzZUxlZ2FjeU1lcmdlU29ydD10cnVlAC1Eam5scC5hcHBsaWNhdGlvbi5ocmVmPWh0dHBzOi8vam9zbS5vcGVuc3RyZWV0bWFwLmRlL2Rvd25sb2FkL2pvc20uam5scAA=]
Dataset consistency test: No problems found

Plugins:
- DirectDownload (31934)
- FixAddresses (31772)
- Mapillary (32040)
- OpeningHoursEditor (31772)
- PicLayer (31895)
- apache-commons (31895)
- apache-http (31895)
- download_along (31772)
- ejml (31895)
- geotools (31895)
- graphview (31895)
- jts (31772)
- measurement (31895)
- ods-bag (0.6.6)
- opendata (32071)
- opendataservices (0.6.6)
- poly (31772)
- reverter (32005)
- scripting (30722)
- turnlanes (31772)
- undelete (31895)
- utilsplugin2 (32018)

Last errors/warnings:
- W: Unable to remove primitives from TestError [tester=MapCSSTagCheckerAndRule [rule=GroupedMapCSSRule [selectors=[*[building][building'NREGEX'no|entrance][ParameterFunction~equal(class java.lang.Object ArrayFunction~any(class java.lang.Object ParameterFunction~tag(class org.openstreetmap.josm.gui.mappaint.Environment <layer>),class java.lang.Object <0>),class java.lang.Object ArrayFunction~any(class java.lang.Object ParameterFunction~parent_tag(class org.openstreetmap.josm.gui.mappaint.Environment <layer>),class java.lang.Object <0>))] >LinkSelector{conditions=null} *[building][building'NREGEX'no|entrance]], declaration=Declaration [instructions=[throwWarning: ArrayFunction~tr(class java.lang.String <Building inside building>);], idx=8]]], code=3000, message=Building inside building]
- W: Unable to remove primitives from TestError [tester=org.openstreetmap.josm.data.validation.tests.CrossingWays$Ways@41069fff, code=601, message=Crossing buildings]
- W: Unable to remove primitives from TestError [tester=org.openstreetmap.josm.data.validation.tests.CrossingWays$Ways@41069fff, code=601, message=Crossing buildings]
- W: Unable to remove primitives from TestError [tester=org.openstreetmap.josm.data.validation.tests.CrossingWays$Ways@41069fff, code=601, message=Crossing buildings]
- E: java.util.ConcurrentModificationException: java.util.ConcurrentModificationException: java.util.ConcurrentModificationException. Cause: java.util.ConcurrentModificationException: java.util.ConcurrentModificationException. Cause: java.util.ConcurrentModificationException

java.util.ConcurrentModificationException: java.util.ConcurrentModificationException: java.util.ConcurrentModificationException
	at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
	at sun.reflect.NativeConstructorAccessorImpl.newInstance(Unknown Source)
	at sun.reflect.DelegatingConstructorAccessorImpl.newInstance(Unknown Source)
	at java.lang.reflect.Constructor.newInstance(Unknown Source)
	at java.util.concurrent.ForkJoinTask.getThrowableException(Unknown Source)
	at java.util.concurrent.ForkJoinTask.reportException(Unknown Source)
	at java.util.concurrent.ForkJoinTask.join(Unknown Source)
	at java.util.concurrent.ForkJoinPool.invoke(Unknown Source)
	at org.openstreetmap.josm.data.osm.visitor.paint.StyledMapRenderer.render(StyledMapRenderer.java:1930)
	at org.openstreetmap.josm.gui.layer.OsmDataLayer.paint(OsmDataLayer.java:401)
	at org.openstreetmap.josm.gui.MapView.paintLayer(MapView.java:643)
	at org.openstreetmap.josm.gui.MapView.paint(MapView.java:727)
	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$4.run(Unknown Source)
	at javax.swing.RepaintManager$4.run(Unknown Source)
	at java.security.AccessController.doPrivileged(Native Method)
	at java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(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$1200(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$500(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$JavaSecurityAccessImpl.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)
...<snip>...

Attachments (1)

patch-fix-12598.patch (2.3 KB) - added by michael2402 4 years ago.

Download all attachments as: .zip

Change History (17)

comment:1 Changed 4 years ago by Don-vip

Component: CoreCore mappaint
Milestone: 16.03

comment:2 Changed 4 years ago by Don-vip

Summary: ConcurrentModificationException after Validation and moving map viewConcurrentModificationException in StyledMapRenderer.render after validation and moving map view

comment:3 Changed 4 years ago by Don-vip

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

comment:4 Changed 4 years ago by Don-vip

Cc: simon04 bastiK added
Keywords: fork join multi thread exception added

More complete stacktrace from #12645:

java.util.ConcurrentModificationException: java.util.ConcurrentModificationException: java.util.ConcurrentModificationException
	at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
	at sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
	at sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
	at java.lang.reflect.Constructor.newInstance(Constructor.java:423)
	at java.util.concurrent.ForkJoinTask.getThrowableException(ForkJoinTask.java:593)
	at java.util.concurrent.ForkJoinTask.reportException(ForkJoinTask.java:677)
	at java.util.concurrent.ForkJoinTask.join(ForkJoinTask.java:720)
	at java.util.concurrent.ForkJoinPool.invoke(ForkJoinPool.java:2616)
	at org.openstreetmap.josm.data.osm.visitor.paint.StyledMapRenderer.render(StyledMapRenderer.java:1930)
	at org.openstreetmap.josm.gui.layer.OsmDataLayer.paint(OsmDataLayer.java:401)
	at org.openstreetmap.josm.gui.MapView.paintLayer(MapView.java:630)
	at org.openstreetmap.josm.gui.MapView.paint(MapView.java:690)
	at javax.swing.JComponent.paintChildren(JComponent.java:889)
	at javax.swing.JComponent.paint(JComponent.java:1065)
	at javax.swing.JComponent.paintToOffscreen(JComponent.java:5210)
	at javax.swing.BufferStrategyPaintManager.paint(BufferStrategyPaintManager.java:290)
	at javax.swing.RepaintManager.paint(RepaintManager.java:1272)
	at javax.swing.JComponent._paintImmediately(JComponent.java:5158)
	at javax.swing.JComponent.paintImmediately(JComponent.java:4969)
	at javax.swing.RepaintManager$4.run(RepaintManager.java:831)
	at javax.swing.RepaintManager$4.run(RepaintManager.java:814)
	at java.security.AccessController.doPrivileged(Native Method)
	at java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:76)
	at javax.swing.RepaintManager.paintDirtyRegions(RepaintManager.java:814)
	at javax.swing.RepaintManager.paintDirtyRegions(RepaintManager.java:789)
	at javax.swing.RepaintManager.prePaintDirtyRegions(RepaintManager.java:738)
	at javax.swing.RepaintManager.access$1200(RepaintManager.java:64)
	at javax.swing.RepaintManager$ProcessingRunnable.run(RepaintManager.java:1732)
	at java.awt.event.InvocationEvent.dispatch(InvocationEvent.java:311)
	at java.awt.EventQueue.dispatchEventImpl(EventQueue.java:756)
	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:76)
	at java.awt.EventQueue.dispatchEvent(EventQueue.java:726)
	at java.awt.EventDispatchThread.pumpOneEventForFilters(EventDispatchThread.java:201)
	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)
Caused by: java.util.ConcurrentModificationException: java.util.ConcurrentModificationException
	at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
	at sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
	at sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
	at java.lang.reflect.Constructor.newInstance(Constructor.java:423)
	at java.util.concurrent.ForkJoinTask.getThrowableException(ForkJoinTask.java:593)
	at java.util.concurrent.ForkJoinTask.reportException(ForkJoinTask.java:677)
	at java.util.concurrent.ForkJoinTask.join(ForkJoinTask.java:720)
	at org.openstreetmap.josm.data.osm.visitor.paint.StyledMapRenderer$ComputeStyleListWorker.compute(StyledMapRenderer.java:1829)
	at org.openstreetmap.josm.data.osm.visitor.paint.StyledMapRenderer$ComputeStyleListWorker.compute(StyledMapRenderer.java:1793)
	at java.util.concurrent.RecursiveTask.exec(RecursiveTask.java:94)
	at java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:289)
	at java.util.concurrent.ForkJoinPool$WorkQueue.runTask(ForkJoinPool.java:1056)
	at java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1692)
	at java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:157)
Caused by: java.util.ConcurrentModificationException
	at java.util.ArrayList$Itr.checkForComodification(ArrayList.java:901)
	at java.util.ArrayList$Itr.next(ArrayList.java:851)
	at java.util.AbstractList.equals(AbstractList.java:522)
	at java.util.Objects.equals(Objects.java:59)
	at org.openstreetmap.josm.gui.mappaint.DividedScale.equals(DividedScale.java:182)
	at java.util.Arrays.deepEquals0(Arrays.java:4299)
...<snip>...

comment:5 Changed 4 years ago by Don-vip

Side effect of r9371?

comment:6 in reply to:  5 Changed 4 years ago by bastiK

Replying to Don-vip:

Side effect of r9371?

I don't think so as r9371 is more or less a cosmetic change + adds robustness against NPE.

This error is really interesting, as DividedScale is an immutable class. It is counter-intuitive that looping over an ArrayList field of an immutable object should throw ConcurrentModificationException. If it's not a stupid typo or coding error, then there must be some kind of advanced compiler optimization. A reference to an object would be visible to another thread before the creation of the object is finished in the current thread. Maybe I have to read up on Java memory model. :-)

comment:7 Changed 4 years ago by Don-vip

Milestone: 16.0316.04

Milestone renamed

comment:8 Changed 4 years ago by michael2402

Cc: michael2402 added

comment:9 Changed 4 years ago by michael2402

DividedScale is not immutable. I made putImpl private, now it is.

Changed 4 years ago by michael2402

Attachment: patch-fix-12598.patch added

comment:10 Changed 4 years ago by bastiK

In 10145/josm:

see #12598 - make DividedScale truely immutable (patch by michael2402)

comment:11 Changed 4 years ago by bastiK

Nice catch, thanks!

If I'm not mistaken, this patch makes the code better to understand, but doesn't have a practical effect on the error in the stacktrace. (I.e. if the code works now, it should have worked before.)

comment:12 Changed 4 years ago by michael2402

The important line in put() is this one:

DividedScale<T> s = new DividedScale<>(this);

It creates the copy. When just using putImpl() you are modifying the old object.

comment:13 Changed 4 years ago by bastiK

Sure, but DividedScale is only used as part of StyleCache, so if StyleCache is immutable, all is fine. Before r10145, StyleCache.put did modify a DividedScale object, but it was a newly created object which isn't shared with the outside before the method returns. The current code effectively executes the same commands, but shifts the modifying bits to the subclass. It shouldn't change the semantics of StyleCache.put as seen from the caller or a concurrent thread.

comment:14 Changed 4 years ago by michael2402

The StyleCache(StyleCache) constructor does not copy the DividedScale. So:

s = new StyleCache()
t = new StyleCache(x) <- share same DividedScale reference
t.put(xxx) <- We would modify the s object here.

comment:15 Changed 4 years ago by bastiK

Resolution: fixed
Status: newclosed

Thanks, now I get it! So it is quite possible that your patch fixes the error.

comment:16 Changed 4 years ago by bastiK

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

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.