Context Navigation

Modify ↓

#23203 closed defect (fixed)

[PATCH] Simplify Way dialog can cause a deadlock

Reported by:	jim.walseth@…	Owned by:	team
Priority:	normal	Milestone:	23.11
Component:	Core	Version:	latest
Keywords:	template_report, simplify way	Cc:

Description

What steps will reproduce the problem?

Open the attached file "SimplifyCrash.osm" using JOSM.app on Mac (Ventura, 13.5.2)
Select the cluster of 5 buildings in the center (screenshot attached).
Use Shift+Y to launch simplification dialog.

What is the expected result?

The dialog will 'preview' how many nodes will be removed. Clicking Simplify removes them.

What happens instead?

The dialog is unresponsive. I have to "force quit" JOSM.

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

Revision:18822
Build-Date:2023-08-30 11:52:18

Identification: JOSM/1.5 (18822 en) Mac OS X 13.5.2
OS Build number: macOS 13.5.2 (22G91)
Memory Usage: 454 MB / 4096 MB (248 MB allocated, but free)
Java version: 17.0.8+7-LTS, Azul Systems, Inc., OpenJDK 64-Bit Server VM
Look and Feel: com.apple.laf.AquaLookAndFeel
Screen: Display 69734272 1440×900 (scaling 2.00×2.00) Display 722470485 1920×1080 (scaling 1.00×1.00) Display 724048470 1920×1200 (scaling 1.00×1.00)
Maximum Screen Size: 1920×1200
Best cursor sizes: 16×16→16×16, 32×32→32×32
System property file.encoding: UTF-8
System property sun.jnu.encoding: UTF-8
Locale info: en_US
Numbers with default locale: 1234567890 -> 1234567890
VM arguments: [-Djpackage.app-version=18822, --add-modules=java.scripting,java.sql,javafx.controls,javafx.media,javafx.swing,javafx.web, --add-exports=java.base/sun.security.action=ALL-UNNAMED, --add-exports=java.desktop/com.apple.eawt=ALL-UNNAMED, --add-exports=java.desktop/com.sun.imageio.plugins.jpeg=ALL-UNNAMED, --add-exports=java.desktop/com.sun.imageio.spi=ALL-UNNAMED, --add-opens=java.base/java.lang=ALL-UNNAMED, --add-opens=java.base/java.nio=ALL-UNNAMED, --add-opens=java.base/jdk.internal.loader=ALL-UNNAMED, --add-opens=java.base/jdk.internal.ref=ALL-UNNAMED, --add-opens=java.desktop/javax.imageio.spi=ALL-UNNAMED, --add-opens=java.desktop/javax.swing.text.html=ALL-UNNAMED, --add-opens=java.prefs/java.util.prefs=ALL-UNNAMED, -Djpackage.app-path=/Applications/JOSM.app/Contents/MacOS/JOSM]

Plugins:
+ buildings_tools (36126)
+ mapathoner (1.5.2)
+ scripting (v0.2.10)
+ todo (133)
+ utilsplugin2 (36126)

Map paint styles:
+ ${HOME}/Downloads/josm_styles-master.zip
- https://josm.openstreetmap.de/josmfile?page=Styles/SimpleBuildingTags&zip=1
- https://josm.openstreetmap.de/josmfile?page=Styles/SimpleRoofTags&zip=1
+ https://github.com/osmlab/appledata/archive/josm_paint_inline_validation.zip
+ https://github.com/MissingMaps/josm_styles/archive/master.zip
+ https://github.com/osmlab/applepaintstyles/archive/main.zip
- https://gitlab.com/peculiar-theater/mapcss/-/archive/main/mapcss-main.zip?path=osm-meta
- https://josm.openstreetmap.de/josmfile?page=Styles/ColorWays&zip=1

Last errors/warnings:
- 00004.481 W: Failed to open file with extension 'mapcss' and namepart 'style' in zip file 'josm_styles-master.zip'. Exception was: java.nio.file.NoSuchFileException: ${HOME}/Downloads/josm_styles-master.zip: java.nio.file.NoSuchFileException: ${HOME}/Downloads/josm_styles-master.zip
- 00004.494 W: Failed to load Mappaint styles from '${HOME}/Downloads/josm_styles-master.zip'. Exception was: java.nio.file.NoSuchFileException: ${HOME}/Downloads/josm_styles-master.zip
- 00004.494 E: java.nio.file.NoSuchFileException: ${HOME}/Downloads/josm_styles-master.zip
- 00004.539 W: Initializing map style ${HOME}/Downloads/josm_styles-master.zip completed in 44 ms (1 errors, 0 warnings)

Attachments (4)

SimplifyCrash.osm (60.4 KB ) - added by jim.walseth@… 14 months ago.
Screenshot 2023-09-26 at 1.37.21 PM.png (70.4 KB ) - added by jim.walseth@… 14 months ago.: screenshot of the crash/hang
23203.patch (2.6 KB ) - added by taylor.smock 14 months ago.
23203.2.patch (9.7 KB ) - added by taylor.smock 14 months ago.: Handle other threads changing selection better

Download all attachments as: .zip

Change History (13)

by jim.walseth@…, 14 months ago

Attachment:	SimplifyCrash.osm added

by jim.walseth@…, 14 months ago

Attachment:	Screenshot 2023-09-26 at 1.37.21 PM.png added

screenshot of the crash/hang

comment:1 by taylor.smock, 14 months ago

I think I found the problem. It doesn't occur reliably, but it looks like we are getting a write lock in SimplifyWayAction.actionPerformed, then a read lock in the same thread in StyledMapRenderer.render (which is fine). The problem is that we then create a ComputeStyleListWorker and run it in a different thread, which then wants to get a read lock in DataSet.search(Nodes|Ways|Relations), but cannot since the EDT thread has a write lock.

@jim.walseth: I was unable to reproduce without adding the paintstyles. As a workaround until I get this fixed and you update to the fixed version, you can try disabling the additional paintstyles (inline validation, missing maps, and the apple paint style) while simplifying ways.

"AWT-EventQueue-0@3285" tid=0x27 nid=NA waiting
  java.lang.Thread.State: WAITING
	  at jdk.internal.misc.Unsafe.park(Unsafe.java:-1)
	  at java.util.concurrent.locks.LockSupport.park(LockSupport.java:371)
	  at java.util.concurrent.ForkJoinTask.awaitDone(ForkJoinTask.java:461)
	  at java.util.concurrent.ForkJoinTask.join(ForkJoinTask.java:651)
	  at java.util.concurrent.ForkJoinPool.invoke(ForkJoinPool.java:2822)
	  at org.openstreetmap.josm.data.osm.visitor.paint.StyledMapRenderer.paintWithLock(StyledMapRenderer.java:1676)
	  at org.openstreetmap.josm.data.osm.visitor.paint.StyledMapRenderer.render(StyledMapRenderer.java:1645)
	  at org.openstreetmap.josm.gui.layer.OsmDataLayer.paint(OsmDataLayer.java:587)
	  at org.openstreetmap.josm.gui.layer.AbstractMapViewPaintable$CompatibilityModeLayerPainter.paint(AbstractMapViewPaintable.java:27)
	  at org.openstreetmap.josm.gui.MapView.paintLayer(MapView.java:476)
	  at org.openstreetmap.josm.gui.MapView.drawMapContent(MapView.java:591)
	  at org.openstreetmap.josm.gui.MapView.paint(MapView.java:498)
	  at javax.swing.JComponent.paintChildren(JComponent.java:961)
	  - locked <0x2615> (a java.awt.Component$AWTTreeLock)
	  at javax.swing.JComponent.paint(JComponent.java:1137)
	  at javax.swing.JComponent.paintToOffscreen(JComponent.java:5318)
	  at javax.swing.RepaintManager$PaintManager.paintDoubleBufferedImpl(RepaintManager.java:1656)
	  at javax.swing.RepaintManager$PaintManager.paintDoubleBuffered(RepaintManager.java:1631)
	  at javax.swing.RepaintManager$PaintManager.paint(RepaintManager.java:1569)
	  at javax.swing.RepaintManager.paint(RepaintManager.java:1336)
	  at javax.swing.JComponent._paintImmediately(JComponent.java:5266)
	  at javax.swing.JComponent.paintImmediately(JComponent.java:5076)
	  at javax.swing.RepaintManager$4.run(RepaintManager.java:878)
	  at javax.swing.RepaintManager$4.run(RepaintManager.java:861)
	  at java.security.AccessController.executePrivileged(AccessController.java:778)
	  at java.security.AccessController.doPrivileged(AccessController.java:400)
	  at java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:87)
	  at javax.swing.RepaintManager.paintDirtyRegions(RepaintManager.java:861)
	  at javax.swing.RepaintManager.paintDirtyRegions(RepaintManager.java:834)
	  at javax.swing.RepaintManager.prePaintDirtyRegions(RepaintManager.java:784)
	  at javax.swing.RepaintManager$ProcessingRunnable.run(RepaintManager.java:1897)
	  at java.awt.event.InvocationEvent.dispatch$$$capture(InvocationEvent.java:318)
	  at java.awt.event.InvocationEvent.dispatch(InvocationEvent.java:-1)
	  at java.awt.EventQueue.dispatchEventImpl(EventQueue.java:773)
	  at java.awt.EventQueue$4.run(EventQueue.java:720)
	  at java.awt.EventQueue$4.run(EventQueue.java:714)
	  at java.security.AccessController.executePrivileged(AccessController.java:778)
	  at java.security.AccessController.doPrivileged(AccessController.java:400)
	  at java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:87)
	  at java.awt.EventQueue.dispatchEvent(EventQueue.java:742)
	  at java.awt.EventDispatchThread.pumpOneEventForFilters(EventDispatchThread.java:203)
	  at java.awt.EventDispatchThread.pumpEventsForFilter(EventDispatchThread.java:124)
	  at java.awt.EventDispatchThread.pumpEventsForFilter(EventDispatchThread.java:117)
	  at java.awt.WaitDispatchSupport$2.run(WaitDispatchSupport.java:191)
	  at java.awt.WaitDispatchSupport$4.run(WaitDispatchSupport.java:236)
	  at java.awt.WaitDispatchSupport$4.run(WaitDispatchSupport.java:234)
	  at java.security.AccessController.executePrivileged(AccessController.java:778)
	  at java.security.AccessController.doPrivileged(AccessController.java:319)
	  at java.awt.WaitDispatchSupport.enter(WaitDispatchSupport.java:234)
	  at java.awt.Dialog.show(Dialog.java:1079)
	  at java.awt.Component.show(Component.java:1728)
	  at java.awt.Component.setVisible(Component.java:1675)
	  at java.awt.Window.setVisible(Window.java:1036)
	  at java.awt.Dialog.setVisible(Dialog.java:1015)
	  at org.openstreetmap.josm.gui.ExtendedDialog.setVisible(ExtendedDialog.java:462)
	  at org.openstreetmap.josm.gui.ExtendedDialog.showDialog(ExtendedDialog.java:258)
	  at org.openstreetmap.josm.actions.SimplifyWayAction.askSimplifyWays(SimplifyWayAction.java:183)
	  at org.openstreetmap.josm.actions.SimplifyWayAction.lambda$actionPerformed$2(SimplifyWayAction.java:220)
	  at org.openstreetmap.josm.actions.SimplifyWayAction$$Lambda/0x000000012e70fb50.run(Unknown Source:-1)
	  at org.openstreetmap.josm.data.osm.DataSet.update(DataSet.java:1063)
	  at org.openstreetmap.josm.actions.SimplifyWayAction.actionPerformed(SimplifyWayAction.java:206)
          [...]

"styled-map-renderer-1@9648" tid=0x51 nid=NA waiting
  java.lang.Thread.State: WAITING
	  at jdk.internal.misc.Unsafe.park(Unsafe.java:-1)
	  at java.util.concurrent.locks.LockSupport.park(LockSupport.java:221)
	  at java.util.concurrent.locks.AbstractQueuedSynchronizer.acquire(AbstractQueuedSynchronizer.java:754)
	  at java.util.concurrent.locks.AbstractQueuedSynchronizer.acquireShared(AbstractQueuedSynchronizer.java:1079)
	  at java.util.concurrent.locks.ReentrantReadWriteLock$ReadLock.lock(ReentrantReadWriteLock.java:738)
	  at org.openstreetmap.josm.data.osm.DataSet.searchNodes(DataSet.java:410)
	  at org.openstreetmap.josm.gui.mappaint.mapcss.Selector$ChildOrParentSelector.visitBBox(Selector.java:505)
	  at org.openstreetmap.josm.gui.mappaint.mapcss.Selector$ChildOrParentSelector.matches(Selector.java:542)
	  at org.openstreetmap.josm.gui.mappaint.mapcss.MapCSSStyleSource.apply(MapCSSStyleSource.java:370)
	  at org.openstreetmap.josm.gui.mappaint.ElemStyles.generateStyles(ElemStyles.java:398)
	  at org.openstreetmap.josm.gui.mappaint.ElemStyles.getImpl(ElemStyles.java:261)
	  at org.openstreetmap.josm.gui.mappaint.ElemStyles.getStyleCacheWithRange(ElemStyles.java:173)
	  - locked <0x2619> (a org.openstreetmap.josm.data.osm.Way)
	  at org.openstreetmap.josm.gui.mappaint.ElemStyles.get(ElemStyles.java:150)
	  at org.openstreetmap.josm.data.osm.visitor.paint.ComputeStyleListWorker.add(ComputeStyleListWorker.java:171)
	  at org.openstreetmap.josm.data.osm.visitor.paint.ComputeStyleListWorker.visit(ComputeStyleListWorker.java:143)
	  at org.openstreetmap.josm.data.osm.Way.accept(Way.java:184)
	  at org.openstreetmap.josm.data.osm.visitor.paint.ComputeStyleListWorker.acceptDrawable(ComputeStyleListWorker.java:129)
	  at org.openstreetmap.josm.data.osm.visitor.paint.ComputeStyleListWorker.computeDirectly(ComputeStyleListWorker.java:116)
	  at org.openstreetmap.josm.data.osm.visitor.paint.ComputeStyleListWorker.compute(ComputeStyleListWorker.java:93)
	  at org.openstreetmap.josm.data.osm.visitor.paint.ComputeStyleListWorker.compute(ComputeStyleListWorker.java:34)
          [...]

by taylor.smock, 14 months ago

Attachment:	23203.patch added

comment:2 by taylor.smock, 14 months ago

Milestone:	→ 23.09

comment:3 by taylor.smock, 14 months ago

Keywords:	simplify way added
Summary:	Simplification of buildings can cause a crash → Simplify Way dialog can cause a deadlock

comment:4 by jwalseth, 14 months ago

Way to jump on this Taylor, thank you! Feels exactly like a deadlock. Perhaps I will be able to go back to my practice of selecting many buildings and simplifying them en masse.

comment:5 by taylor.smock, 14 months ago

I've been playing around with the code a bit, and I think I'm going to have to push this off to milestone:"23.10". I'm doing some defensive programing to deal with some other code changing the selection (or deleting a selected way) while people have the simplify way dialog open.

Right now, I don't think we have any code that does that, but I want to avoid issues in the future.

comment:6 by taylor.smock, 14 months ago

Milestone:	23.09 → 23.10
Summary:	Simplify Way dialog can cause a deadlock → [PATCH] Simplify Way dialog can cause a deadlock

by taylor.smock, 14 months ago

Attachment:	23203.2.patch added

Handle other threads changing selection better

comment:7 by taylor.smock, 14 months ago

Resolution:	→ fixed
Status:	new → closed

In 18851/josm:

Fix #23203: Simplify Way dialog can cause a deadlock

This is caused by a write lock being acquired in SimplifyWayAction.actionPerformed
in the EDT and then a read lock is attempted to be acquired in a styled-map-renderer
thread. The styled-map-renderer thread is waited on by the EDT, which causes the
deadlock. The fix is removing the dataset write lock in SimplifyWayAction.actionPerformed
which was added back in 2011. An analysis of the code shows that the commands generated
lock the dataset themselves, so the dataset lock in SimplifyWayAction.actionPerformed
should no longer be necessary.

The deadlock does not happen reliably with only the JOSM default paintstyle; adding
additional paintstyles was required for the initial analysis.

comment:8 by taylor.smock, 13 months ago

Milestone:	23.10 → 23.11

Ticket retargeted after milestone deleted

comment:9 by GerdP, 11 months ago

See #23399 for a regression.

Modify Ticket

Change Properties

Summary:
Type:		Priority:
Milestone:		Component:
Version:		Keywords:
Cc:	Set your email in Preferences

Action

leave as closed The owner will remain team.

change resolution as The resolution will be set.

reopen The resolution will be deleted. Next status will be 'reopened'.

Add Comment

Your email or username:

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

You may use WikiFormatting here.

Attachments ↑ Description ↑

Note: See TracTickets for help on using tickets.

Download in other formats:

Context Navigation

#23203 closed defect (fixed)

[PATCH] Simplify Way dialog can cause a deadlock

Description

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.

Attachments (4)

Change History (13)

by jim.walseth@…, 14 months ago

by jim.walseth@…, 14 months ago

comment:1 by taylor.smock, 14 months ago

by taylor.smock, 14 months ago

comment:2 by taylor.smock, 14 months ago

comment:3 by taylor.smock, 14 months ago

comment:4 by jwalseth, 14 months ago

comment:5 by taylor.smock, 14 months ago

comment:6 by taylor.smock, 14 months ago

by taylor.smock, 14 months ago

comment:7 by taylor.smock, 14 months ago

comment:8 by taylor.smock, 13 months ago

comment:9 by GerdP, 11 months ago

Modify Ticket

Add Comment

by anonymous

Download in other formats: