Fix for Bug 277966 [Databinding] regression: ObservablesManager throws NPE on dispose if one of the observables is already disposedI20090603-2000I20090602-2000I20090528-2000

2 files changed, 18 insertions, 4 deletions

diff --git a/bundles/org.eclipse.core.databinding/src/org/eclipse/core/databinding/ObservablesManager.java b/bundles/org.eclipse.core.databinding/src/org/eclipse/core/databinding/ObservablesManager.java
index a1da658c..dc20358c 100644
--- a/bundles/org.eclipse.core.databinding/src/org/eclipse/core/databinding/ObservablesManager.java
+++ b/bundles/org.eclipse.core.databinding/src/org/eclipse/core/databinding/ObservablesManager.java
@@ -14,13 +14,13 @@
 package org.eclipse.core.databinding;
 
 import java.util.HashMap;
-import java.util.HashSet;
 import java.util.Iterator;
 import java.util.Map;
 import java.util.Set;
 
 import org.eclipse.core.databinding.observable.IObservable;
 import org.eclipse.core.databinding.observable.ObservableTracker;
+import org.eclipse.core.internal.databinding.IdentitySet;
 import org.eclipse.core.internal.databinding.Pair;
 
 /**
@@ -34,8 +34,8 @@ import org.eclipse.core.internal.databinding.Pair;
  */
 public class ObservablesManager {
 
-	private Set managedObservables = new HashSet();
-	private Set excludedObservables = new HashSet();
+	private Set managedObservables = new IdentitySet();
+	private Set excludedObservables = new IdentitySet();
 	private Map contexts = new HashMap();
 
 	/**
@@ -104,7 +104,7 @@ public class ObservablesManager {
 	 * Disposes of this manager and all observables that it manages.
 	 */
 	public void dispose() {
-		Set observables = new HashSet();
+		Set observables = new IdentitySet();
 		observables.addAll(managedObservables);
 		for (Iterator it = contexts.keySet().iterator(); it.hasNext();) {
 			DataBindingContext context = (DataBindingContext) it.next();
diff --git a/tests/org.eclipse.jface.tests.databinding/src/org/eclipse/core/tests/databinding/ObservablesManagerTest.java b/tests/org.eclipse.jface.tests.databinding/src/org/eclipse/core/tests/databinding/ObservablesManagerTest.java
index b7d1d49a..4c332f05 100644
--- a/tests/org.eclipse.jface.tests.databinding/src/org/eclipse/core/tests/databinding/ObservablesManagerTest.java
+++ b/tests/org.eclipse.jface.tests.databinding/src/org/eclipse/core/tests/databinding/ObservablesManagerTest.java
@@ -16,8 +16,10 @@ import org.eclipse.core.databinding.DataBindingContext;
 import org.eclipse.core.databinding.ObservablesManager;
 import org.eclipse.core.databinding.observable.IChangeListener;
 import org.eclipse.core.databinding.observable.IDisposeListener;
+import org.eclipse.core.databinding.observable.IObservable;
 import org.eclipse.core.databinding.observable.IStaleListener;
 import org.eclipse.core.databinding.observable.Realm;
+import org.eclipse.core.databinding.observable.list.WritableList;
 import org.eclipse.core.databinding.observable.value.IObservableValue;
 import org.eclipse.core.databinding.observable.value.IValueChangeListener;
 import org.eclipse.jface.tests.databinding.AbstractDefaultRealmTestCase;
@@ -98,6 +100,18 @@ public class ObservablesManagerTest extends AbstractDefaultRealmTestCase {
 		assertTrue(modelOv.disposeCalled);
 	}
 
+	public void testDispose_Bug277966_NPEWhenManagedObservableAlreadyDisposed() {
+		ObservablesManager manager = new ObservablesManager();
+
+		// NPE only occurs when explicitly managing (i.e. not through a
+		// DataBindingContext) observables where hashCode() is a tracked getter
+		IObservable observable = new WritableList();
+		manager.addObservable(observable);
+		observable.dispose();
+
+		manager.dispose();
+	}
+
 	private static class FlagOnDisposeObservableValue implements
 			IObservableValue {