Changeset 3584 in josm

Timestamp:

Oct 3, 2010 3:25:36 PM (3 years ago)

Author:

jttt

Message:

Fix #5525 Error when adding a node in the middle of a way

Location:

trunk/src/org/openstreetmap/josm/data/osm

Files:

• DataSet.java (modified) (3 diffs)
• QuadBuckets.java (modified) (8 diffs)

trunk/src/org/openstreetmap/josm/data/osm/DataSet.java

@@ -152 +152 @@
 
     public List<Node> searchNodes(BBox bbox) {
-        return nodes.search(bbox);
+        lock.readLock().lock();
+        try {
+            return nodes.search(bbox);
+        } finally {
+            lock.readLock().unlock();
+        }
     }
 
@@ -172 +177 @@
 
     public List<Way> searchWays(BBox bbox) {
-        return ways.search(bbox);
+        lock.readLock().lock();
+        try {
+            return ways.search(bbox);
+        } finally {
+            lock.readLock().unlock();
+        }
     }
 
@@ -190 +200 @@
 
     public List<Relation> searchRelations(BBox bbox) {
-        // QuadBuckets might be useful here (don't forget to do reindexing after some of rm is changed)
-        List<Relation> result = new ArrayList<Relation>();
-        for (Relation r: relations) {
-            if (r.getBBox().intersects(bbox)) {
-                result.add(r);
-            }
-        }
-        return result;
+        lock.readLock().lock();
+        try {
+            // QuadBuckets might be useful here (don't forget to do reindexing after some of rm is changed)
+            List<Relation> result = new ArrayList<Relation>();
+            for (Relation r: relations) {
+                if (r.getBBox().intersects(bbox)) {
+                    result.add(r);
+                }
+            }
+            return result;
+        } finally {
+            lock.readLock().unlock();
+        }
     }
 

trunk/src/org/openstreetmap/josm/data/osm/QuadBuckets.java

@@ -14 +14 @@
  * Note: bbox of primitives added to QuadBuckets has to stay the same. In case of coordinate change, primitive must
  * be removed and readded.
+ *
+ * This class is (no longer) thread safe.
  *
  */
@@ -24 +26 @@
     private static final int SE_INDEX = 2;
     private static final int SW_INDEX = 0;
-    /*
-     * Functions prefixed with __ need locking before
-     * being called.
-     */
-    private final Object split_lock = new Object();
 
     static void abort(String s)
@@ -169 +166 @@
         boolean remove_content(T o)
         {
-            synchronized (split_lock) {
-                // If two threads try to remove item at the same time from different buckets of this QBLevel,
-                // it might happen that one thread removes bucket but don't remove parent because it still sees
-                // another bucket set. Second thread do the same. Due to thread memory caching, it's possible that
-                // changes made by threads will show up in children array too late, leading to QBLevel with all children
-                // set to null
-                if (content == null)
-                    return false;
-                boolean ret = this.content.remove(o);
-                if (this.content.size() == 0) {
-                    this.content = null;
-                }
-                if (this.canRemove()) {
-                    this.remove_from_parent();
-                }
-                return ret;
-            }
+            // If two threads try to remove item at the same time from different buckets of this QBLevel,
+            // it might happen that one thread removes bucket but don't remove parent because it still sees
+            // another bucket set. Second thread do the same. Due to thread memory caching, it's possible that
+            // changes made by threads will show up in children array too late, leading to QBLevel with all children
+            // set to null
+            if (content == null)
+                return false;
+            boolean ret = this.content.remove(o);
+            if (this.content.size() == 0) {
+                this.content = null;
+            }
+            if (this.canRemove()) {
+                this.remove_from_parent();
+            }
+            return ret;
         }
         // Get the correct index for the given primitive
@@ -394 +389 @@
                 }
             }
-            synchronized (split_lock) {
-                __add_content(o);
-                if (isLeaf() && content.size() > MAX_OBJECTS_PER_LEVEL && level < QuadTiling.NR_LEVELS) {
-                    __split();
-                }
+            __add_content(o);
+            if (isLeaf() && content.size() > MAX_OBJECTS_PER_LEVEL && level < QuadTiling.NR_LEVELS) {
+                __split();
             }
         }
 
         void add(T o) {
-            synchronized (split_lock) {
-                findBucket(o.getBBox()).doAdd(o);
-            }
+            findBucket(o.getBBox()).doAdd(o);
         }
 
@@ -529 +520 @@
     }
     public void clear()  {
-        synchronized (split_lock) {
-            root = new QBLevel();
-            search_cache = null;
-            size = 0;
-            if (debug) {
-                out("QuadBuckets() cleared: " + this);
-                out("root: " + root + " level: " + root.level + " bbox: " + root.bbox());
-            }
+        root = new QBLevel();
+        search_cache = null;
+        size = 0;
+        if (debug) {
+            out("QuadBuckets() cleared: " + this);
+            out("root: " + root + " level: " + root.level + " bbox: " + root.bbox());
         }
     }
     public boolean add(T n) {
-        synchronized (split_lock) {
-            root.add(n);
-            size++;
-            return true;
-        }
-    }
+        root.add(n);
+        size++;
+        return true;
+    }
+
     public void unsupported()
     {
@@ -588 +576 @@
     public boolean remove(Object o) {
         @SuppressWarnings("unchecked") T t = (T) o;
-        synchronized (split_lock) {
-            search_cache = null; // Search cache might point to one of removed buckets
-            QBLevel bucket = root.findBucket(t.getBBox());
-            if (bucket.remove_content(t)) {
-                size--;
-                return true;
-            } else
-                return false;
-        }
+        search_cache = null; // Search cache might point to one of removed buckets
+        QBLevel bucket = root.findBucket(t.getBBox());
+        if (bucket.remove_content(t)) {
+            size--;
+            return true;
+        } else
+            return false;
     }
     public boolean contains(Object o) {
@@ -634 +620 @@
             QBLevel orig = q;
             QBLevel next;
-            synchronized (split_lock) {
-                next = q.nextContentNode();
-            }
+            next = q.nextContentNode();
             //if (consistency_testing && (orig == next))
             if (orig == next) {
@@ -726 +710 @@
     }
     public int size() {
-        synchronized (split_lock) {
-            return size;
-        }
+        return size;
     }