Opened 13 years ago

Last modified 13 years ago

#7938 closed defect

[critical/blocking] stack overflow: infinite exceptions when opening Conflict editor on a relation — at Version 7

Reported by: verdy_p Owned by: team
Priority: critical Milestone:
Component: Core Version:
Keywords: crash Cc:

Description (last modified by bastiK)

java.lang.StackOverflowError
	at sun.java2d.pipe.AlphaColorPipe.drawParallelogram(Unknown Source)
	at sun.java2d.pipe.PixelToParallelogramConverter.drawRectangle(Unknown Source)
	at sun.java2d.pipe.PixelToParallelogramConverter.drawRect(Unknown Source)
	at sun.java2d.SunGraphics2D.drawRect(Unknown Source)
	at org.openstreetmap.josm.gui.dialogs.ConflictDialog$4.visit(ConflictDialog.java:188)
	at org.openstreetmap.josm.data.osm.Node.visit(Node.java:184)
	at org.openstreetmap.josm.gui.dialogs.ConflictDialog$4.visit(ConflictDialog.java:208)
	at org.openstreetmap.josm.data.osm.Relation.visit(Relation.java:165)
	at org.openstreetmap.josm.gui.dialogs.ConflictDialog$4.visit(ConflictDialog.java:208)
	at org.openstreetmap.josm.data.osm.Relation.visit(Relation.java:165)
	at org.openstreetmap.josm.gui.dialogs.ConflictDialog$4.visit(ConflictDialog.java:208)
	at org.openstreetmap.josm.data.osm.Relation.visit(Relation.java:165)
	at org.openstreetmap.josm.gui.dialogs.ConflictDialog$4.visit(ConflictDialog.java:208)
	at org.openstreetmap.josm.data.osm.Relation.visit(Relation.java:165)
	at org.openstreetmap.josm.gui.dialogs.ConflictDialog$4.visit(ConflictDialog.java:208)
	at org.openstreetmap.josm.data.osm.Relation.visit(Relation.java:165)
	at org.openstreetmap.josm.gui.dialogs.ConflictDialog$4.visit(ConflictDialog.java:208)
	at org.openstreetmap.josm.data.osm.Relation.visit(Relation.java:165)
	at org.openstreetmap.josm.gui.dialogs.ConflictDialog$4.visit(ConflictDialog.java:208)
	at org.openstreetmap.josm.data.osm.Relation.visit(Relation.java:165)
	at org.openstreetmap.josm.gui.dialogs.ConflictDialog$4.visit(ConflictDialog.java:208)
	at org.openstreetmap.josm.data.osm.Relation.visit(Relation.java:165)
	at org.openstreetmap.josm.gui.dialogs.ConflictDialog$4.visit(ConflictDialog.java:208)
... (infinitely reported... the initial exception cause is not logged, even in the Java console, due to the stack overflow)

Then JOSM freezes and no longer refreshes its window (and even the Java console freezes)

Certainly a synchronization/serialization issue with the GUI thread.

JOSM development version 5406.

Change History (7)

comment:1 by anonymous, 13 years ago

May be also related to Ticket #7936 (also in Conflit editor).

comment:2 by verdy_p, 13 years ago

Visibly this is caused by a way with zero nodes in the conflict editor. I don't why this occured, because the way 105079938 (in Madrid, Spain) was absolutely not within my edit area and is completely unrelated to the conflict to solve (in the French region of Franche-Comté). It seems that references to objects get mixed up of objects with zero nodes (deleted objects) are considered as one and then shared spuriously by many other relations, and the conflict editor gets confused when visiting them (e.g. if there are two references to what were distinct objects, both deleted, from the same relation object being edited).

Note: this also affects the current release version :

Repository Root: http://josm.openstreetmap.de/svn
Build-Date: 2012-07-23 01:31:06
Last Changed Author: Don-vip
Revision: 5356
Repository UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b
URL: http://josm.openstreetmap.de/svn/trunk
Last Changed Date: 2012-07-22 22:36:44 +0200 (Sun, 22 Jul 2012)
Last Changed Rev: 5356

Identification: JOSM/1.5 (5356 fr)
Memory Usage: 1486 MB / 2714 MB (859 MB allocated, but free)
Java version: 1.7.0_05, Oracle Corporation, Java HotSpot(TM) 64-Bit Server VM
Operating system: Windows 7

Dataset consistency test:
[WARN - ZERO NODES] Way {Way id=105079938 version=2 VT nodes=[]} has zero nodes

Plugin: cadastre-fr (28412)
Plugin: epci-fr (26960)
Plugin: licensechange (28412)
Plugin: mapdust (28412)
Plugin: merge-overlap (28412)
Plugin: multipoly-convert (28412)
Plugin: openstreetbugs (28412)
Plugin: osmarender (28412)
Plugin: reltoolbox (28412)
Plugin: reverter (28535)
Plugin: undelete (28501)
Plugin: waydownloader (28412)

java.lang.StackOverflowError

at sun.java2d.pipe.AlphaColorPipe.drawParallelogram(Unknown Source)
at sun.java2d.pipe.PixelToParallelogramConverter.drawRectangle(Unknown Source)
at sun.java2d.pipe.PixelToParallelogramConverter.drawRect(Unknown Source)
at sun.java2d.SunGraphics2D.drawRect(Unknown Source)
at org.openstreetmap.josm.gui.dialogs.ConflictDialog$4.visit(ConflictDialog.java:188)
at org.openstreetmap.josm.data.osm.Node.visit(Node.java:184)
at org.openstreetmap.josm.gui.dialogs.ConflictDialog$4.visit(ConflictDialog.java:208)
at org.openstreetmap.josm.data.osm.Relation.visit(Relation.java:165)
at org.openstreetmap.josm.gui.dialogs.ConflictDialog$4.visit(ConflictDialog.java:208)
at org.openstreetmap.josm.data.osm.Relation.visit(Relation.java:165)

[...the rest is sniped as these two last lines is repeated many times infinitely up to the max log size...]

comment:3 by verdy_p, 13 years ago

I saved My .osm file and it looks like the way affected above was in fact not completely loaded from the server. It has the correct version 2, and the correct timestamp, but it contains only a few tags of that version, but NO <nd ref='...'> member for that version.

In fact a lot of nodes are missing each time : the server seems to NOT send all members when loading a way or a relation. IF a few nodes or members are missing, then we'll have conflicts detected when uploading modified data. But if there remains NO member node at all in a way that was apparetly successfully loaded, this causes an infinite loop of exceptions in JOSM when it tried to visit it.

The consistency checker (run from the bug report alert) correctly detects that, but the conflict editor does not check that there are effectively some nodes in a way and fails when trying to select it on the map, and the map renderer also fails and turns into an infinite loop too.

Unfortunately, I can't find any way to synchronize the OSM file: as soon as I try to select it in order to reload it from the server, JOSM hangs in an infinite loop. If I load that .osm file but do not select anything and then use "load object" to load that missing object by reference, JOSM will detect a conflict and will fail to edit that conflict because the server object and the local object use the SAME version & timestamp.

Ideally:

  • the conflict editor should work work with ways whose ALL nodes are missing for any kind of reasons.
  • the server should generate a data signature to all objects, or at least should indicate, in an XML atttribute of the <way> element the number of member nodes it is supposed to contain (this will allow consistency check on the client side). It should do the same thing for the number of members in relation objects, to make sure that the data loaded from the server was complete and not unexpectedly interrupted before end on the server side.

Objects may have been loaded also at one time where some members were still hidden by the redaction bot, but they have been "resurrected" later by making them visible. Some inconsistant data see to be reported by the server about those resurrected/inhidden objects.

comment:4 by verdy_p, 13 years ago

Hmmmm... After thought the infinite loop comes from the fact that relation 8642 contains a member with role "defaults" that references the relation ref='957702', in which the relation 8642 is also a member (with role "apply_to").

The conflicts resolver attempts to visit the complete tree of all children and subchildren of a relation, without detecting the possible loop of parent relations already visited in the stack !

  <relation id='8642' action='modify' timestamp='2012-06-26T12:49:28Z' uid='13442' user='Damouns' visible='true' version='360' changeset='12022212'>
    <!--(snip)-->
    <member type='relation' ref='957702' role='defaults' /> <!-- !!!!!!! BUG HERE !!!!! -->
    <!--(snip)-->
    <tag k='admin_level' v='4' />
    <tag k='boundary' v='administrative' />
    <tag k='name' v='Franche-Comté' />
    <!--(snip)-->
  </relation>

  <relation id='957702' timestamp='2011-02-01T08:15:07Z' uid='178186' user='mdk' visible='true' version='3' changeset='7152372'>
    <!--(snip)-->
    <member type='relation' ref='8642' role='apply_to' />
    <tag k='name:fr' v='France, vacances scolaires, zone A' />
    <tag k='type' v='defaults' />
    <!--(snip)-->
  </relation>

Note that the relation 8642 v36 was edited by me and someone else that saved his changes in v361 before me, so this raised a version conflict. The conflict resolver however does not support the resolution of such circular references of relations.

What is the validity of such backward reference from a subarea to his parent area ?

comment:5 by verdy_p, 13 years ago

I confirm the infinite loop is caused effectively in these lines:

205                 public void visit(Relation e) {
207	                for (RelationMember em : e.getMembers()) {
208	                    em.getMember().visit(this);
209	                }
210	            }

This method does not check that the em.getMember() will not return an item that is already in the stack normally maintained by this. But there's no trace in this to record such stack, for example you could add "HashedSet<OsmPrimitive> visited;" member in the new anonymous AbstractVisitor subclass created on line 185 of ConflictDialog.java:

Before visiting each member that you get from "OsmPrimitive m = em.getMember();", first check if it's in the "visited" collection, if yes, do nothing; else add "m" into "visited", then you can "m.visit(this)" and finally remove "m" from "visited".

205                 public void visit(Relation e) {
207	                for (RelationMember em : e.getMembers()) {
208                         OsmPrimitive m = em.getMember();
+                           if (m instanceof Node || m instanceof Way) {
+                               m.visit(this);
+                           } else if (!visited.contains(m)) {
+                               visited.add(m);
+                               try {
+                                   m.visit(this);
+                               } finally {
+                                   visited.remove(m);
+                               }
209	                }
210	            }

The caveat is that the generic Visitor model is slow here, especially when there's a conflict on a very large relation containing lots of levels of children relations. Do you need to recurse here ?

  • Only the members of OsmPrimitive types Way and Node may need to be visited here to draw them in conflict for the conflict editor dialog.
  • Other types could be simply ignored: remove the else-if clause in this patch above, then you don't need to manage the hashed set containing the already visited primitives.

comment:6 by stoecker, 13 years ago

Please supply a patch. You actually have currently much deeper insight into this particular part of code than anybody else.

comment:7 by bastiK, 13 years ago

Description: modified (diff)
Note: See TracTickets for help on using tickets.