bug 361905: fix cleanup of bundleId->Region mapping on region removal

3 files changed, 70 insertions, 10 deletions

diff --git a/bundles/org.eclipse.equinox.region/src/org/eclipse/equinox/internal/region/BundleIdToRegionMapping.java b/bundles/org.eclipse.equinox.region/src/org/eclipse/equinox/internal/region/BundleIdToRegionMapping.java
index 865e71e55..a6a3802fc 100644
--- a/bundles/org.eclipse.equinox.region/src/org/eclipse/equinox/internal/region/BundleIdToRegionMapping.java
+++ b/bundles/org.eclipse.equinox.region/src/org/eclipse/equinox/internal/region/BundleIdToRegionMapping.java
@@ -29,8 +29,6 @@ public interface BundleIdToRegionMapping {
 	 * If the bundle id is already associated with the given region, there is no
 	 * effect on the association and no exception is thrown.
 	 * 
-	 * TODO: need to check type of Region? Validity of region?
-	 * 
 	 * @param bundleId the bundle id to be associated
 	 * @param region the {@link Region} with which the bundle id is to be associated
 	 * @throws BundleException
@@ -46,6 +44,12 @@ public interface BundleIdToRegionMapping {
 	void dissociateBundle(long bundleId);
 
 	/**
+	 * Dissociates any bundle ids which may be associated with the given region.
+	 * @param region the {@link Region} to be dissociated
+	 */
+	void dissociateRegion(Region region);
+
+	/**
 	 * Returns the {@link Region} associated with the given bundle id or <code>null</code>
 	 * if the given bundle id is not associated with a region.
 	 * 
diff --git a/bundles/org.eclipse.equinox.region/src/org/eclipse/equinox/internal/region/StandardBundleIdToRegionMapping.java b/bundles/org.eclipse.equinox.region/src/org/eclipse/equinox/internal/region/StandardBundleIdToRegionMapping.java
index 921053582..0a22b88db 100644
--- a/bundles/org.eclipse.equinox.region/src/org/eclipse/equinox/internal/region/StandardBundleIdToRegionMapping.java
+++ b/bundles/org.eclipse.equinox.region/src/org/eclipse/equinox/internal/region/StandardBundleIdToRegionMapping.java
@@ -12,6 +12,7 @@
 package org.eclipse.equinox.internal.region;
 
 import java.util.*;
+import java.util.Map.Entry;
 import org.eclipse.equinox.region.Region;
 import org.osgi.framework.BundleException;
 
@@ -94,4 +95,20 @@ final class StandardBundleIdToRegionMapping implements BundleIdToRegionMapping {
 			return this.bundleToRegion.get(bundleId);
 		}
 	}
+
+	/** 
+	 * {@inheritDoc}
+	 */
+	@Override
+	public void dissociateRegion(Region region) {
+		synchronized (this.monitor) {
+			Iterator<Entry<Long, Region>> iterator = this.bundleToRegion.entrySet().iterator();
+			while (iterator.hasNext()) {
+				Entry<Long, Region> entry = iterator.next();
+				if (entry.getValue() == region) {
+					iterator.remove();
+				}
+			}
+		}
+	}
 }
diff --git a/bundles/org.eclipse.equinox.region/src/org/eclipse/equinox/internal/region/StandardRegionDigraph.java b/bundles/org.eclipse.equinox.region/src/org/eclipse/equinox/internal/region/StandardRegionDigraph.java
index 4837dd8ca..d04c25a0b 100644
--- a/bundles/org.eclipse.equinox.region/src/org/eclipse/equinox/internal/region/StandardRegionDigraph.java
+++ b/bundles/org.eclipse.equinox.region/src/org/eclipse/equinox/internal/region/StandardRegionDigraph.java
@@ -39,6 +39,8 @@ public final class StandardRegionDigraph implements BundleIdToRegionMapping, Reg
 
 	private final Set<Region> regions = new HashSet<Region>();
 
+	// Alien calls may be made to the following object while this.monitor is locked
+	// as this.monitor is higher in the lock hierarchy than this object's own monitor.
 	private final BundleIdToRegionMapping bundleIdToRegionMapping;
 
 	/* edges maps a given region to an immutable set of edges with their tail at the given region. To update
@@ -272,6 +274,7 @@ public final class StandardRegionDigraph implements BundleIdToRegionMapping, Reg
 					}
 				}
 			}
+			this.bundleIdToRegionMapping.dissociateRegion(region);
 			incrementUpdateCount();
 		}
 	}
@@ -496,10 +499,15 @@ public final class StandardRegionDigraph implements BundleIdToRegionMapping, Reg
 		return this.defaultRegion;
 	}
 
+	/** 
+	 * {@inheritDoc}
+	 */
 	@Override
 	public void associateBundleWithRegion(long bundleId, Region region) throws BundleException {
-		this.bundleIdToRegionMapping.associateBundleWithRegion(bundleId, region);
-		incrementUpdateCount();
+		synchronized (this.monitor) {
+			this.bundleIdToRegionMapping.associateBundleWithRegion(bundleId, region);
+			incrementUpdateCount();
+		}
 	}
 
 	private void incrementUpdateCount() {
@@ -509,26 +517,57 @@ public final class StandardRegionDigraph implements BundleIdToRegionMapping, Reg
 
 	}
 
+	/** 
+	 * {@inheritDoc}
+	 */
 	@Override
 	public void dissociateBundle(long bundleId) {
-		this.bundleIdToRegionMapping.dissociateBundle(bundleId);
-		incrementUpdateCount();
+		synchronized (this.monitor) {
+			this.bundleIdToRegionMapping.dissociateBundle(bundleId);
+			incrementUpdateCount();
+		}
 	}
 
+	/** 
+	 * {@inheritDoc}
+	 */
 	@Override
 	public boolean isBundleAssociatedWithRegion(long bundleId, Region region) {
-		return this.bundleIdToRegionMapping.isBundleAssociatedWithRegion(bundleId, region);
+		synchronized (this.monitor) {
+			return this.bundleIdToRegionMapping.isBundleAssociatedWithRegion(bundleId, region);
+		}
 	}
 
+	/** 
+	 * {@inheritDoc}
+	 */
 	@Override
 	public Set<Long> getBundleIds(Region region) {
-		return this.bundleIdToRegionMapping.getBundleIds(region);
+		synchronized (this.monitor) {
+			return this.bundleIdToRegionMapping.getBundleIds(region);
+		}
 	}
 
+	/** 
+	 * {@inheritDoc}
+	 */
 	@Override
 	public void clear() {
-		this.bundleIdToRegionMapping.clear();
-		incrementUpdateCount();
+		synchronized (this.monitor) {
+			this.bundleIdToRegionMapping.clear();
+			incrementUpdateCount();
+		}
+	}
+
+	/** 
+	 * {@inheritDoc}
+	 */
+	@Override
+	public void dissociateRegion(Region region) {
+		synchronized (this.monitor) {
+			this.bundleIdToRegionMapping.dissociateRegion(region);
+			incrementUpdateCount();
+		}
 	}
 
 }