FIXED - bug 349038: [DataBinding] NPE in SimplePropertyObservableList
after disposal
https://bugs.eclipse.org/bugs/show_bug.cgi?id=349038
diff --git a/bundles/org.eclipse.core.databinding.observable/META-INF/MANIFEST.MF b/bundles/org.eclipse.core.databinding.observable/META-INF/MANIFEST.MF
index 1bdf4a1..12b7a1e 100644
--- a/bundles/org.eclipse.core.databinding.observable/META-INF/MANIFEST.MF
+++ b/bundles/org.eclipse.core.databinding.observable/META-INF/MANIFEST.MF
@@ -2,7 +2,7 @@
Bundle-ManifestVersion: 2
Bundle-Name: %pluginName
Bundle-SymbolicName: org.eclipse.core.databinding.observable
-Bundle-Version: 1.4.0.qualifier
+Bundle-Version: 1.4.1.qualifier
Bundle-ClassPath: .
Bundle-Vendor: %providerName
Bundle-Localization: plugin
diff --git a/bundles/org.eclipse.core.databinding.observable/src/org/eclipse/core/databinding/observable/list/AbstractObservableList.java b/bundles/org.eclipse.core.databinding.observable/src/org/eclipse/core/databinding/observable/list/AbstractObservableList.java
index 8641afb..1bd705d 100644
--- a/bundles/org.eclipse.core.databinding.observable/src/org/eclipse/core/databinding/observable/list/AbstractObservableList.java
+++ b/bundles/org.eclipse.core.databinding.observable/src/org/eclipse/core/databinding/observable/list/AbstractObservableList.java
@@ -8,7 +8,8 @@
* Contributors:
* IBM Corporation - initial API and implementation
* Brad Reynolds - bugs 164653, 167204
- * Matthew Hall - bugs 118516, 208858, 208332, 247367, 146397, 249526
+ * Matthew Hall - bugs 118516, 208858, 208332, 247367, 146397, 249526,
+ * 349038
*******************************************************************************/
package org.eclipse.core.databinding.observable.list;
@@ -63,7 +64,7 @@
private final Realm realm;
private PrivateChangeSupport changeSupport;
- private boolean disposed = false;
+ private volatile boolean disposed = false;
/**
* @param realm
@@ -89,8 +90,8 @@
* @return whether this observable list has any registered listeners.
* @since 1.2
*/
- protected boolean hasListeners() {
- return changeSupport.hasListeners();
+ protected synchronized boolean hasListeners() {
+ return !disposed && changeSupport.hasListeners();
}
public boolean isStale() {
@@ -99,13 +100,15 @@
}
public synchronized void addListChangeListener(IListChangeListener listener) {
- if (!disposed)
+ if (!disposed) {
changeSupport.addListener(ListChangeEvent.TYPE, listener);
+ }
}
public synchronized void removeListChangeListener(IListChangeListener listener) {
- if (!disposed)
+ if (!disposed) {
changeSupport.removeListener(ListChangeEvent.TYPE, listener);
+ }
}
protected void fireListChange(ListDiff diff) {
@@ -115,30 +118,34 @@
}
public synchronized void addChangeListener(IChangeListener listener) {
- if (!disposed)
+ if (!disposed) {
changeSupport.addChangeListener(listener);
+ }
}
public synchronized void removeChangeListener(IChangeListener listener) {
- if (!disposed)
+ if (!disposed) {
changeSupport.removeChangeListener(listener);
+ }
}
public synchronized void addStaleListener(IStaleListener listener) {
- if (!disposed)
+ if (!disposed) {
changeSupport.addStaleListener(listener);
+ }
}
public synchronized void removeStaleListener(IStaleListener listener) {
- if (!disposed)
+ if (!disposed) {
changeSupport.removeStaleListener(listener);
+ }
}
/**
* @since 1.2
*/
public synchronized void addDisposeListener(IDisposeListener listener) {
- if (changeSupport != null) {
+ if (!disposed) {
changeSupport.addDisposeListener(listener);
}
}
@@ -147,7 +154,7 @@
* @since 1.2
*/
public synchronized void removeDisposeListener(IDisposeListener listener) {
- if (changeSupport != null) {
+ if (!disposed) {
changeSupport.removeDisposeListener(listener);
}
}
diff --git a/bundles/org.eclipse.core.databinding.observable/src/org/eclipse/core/databinding/observable/map/AbstractObservableMap.java b/bundles/org.eclipse.core.databinding.observable/src/org/eclipse/core/databinding/observable/map/AbstractObservableMap.java
index fddee3e..8d5e35a 100644
--- a/bundles/org.eclipse.core.databinding.observable/src/org/eclipse/core/databinding/observable/map/AbstractObservableMap.java
+++ b/bundles/org.eclipse.core.databinding.observable/src/org/eclipse/core/databinding/observable/map/AbstractObservableMap.java
@@ -8,7 +8,8 @@
* Contributors:
* IBM Corporation - initial API and implementation
* Brad Reynolds - bug 164653
- * Matthew Hall - bugs 118516, 146397, 226289, 246103, 249526, 264307
+ * Matthew Hall - bugs 118516, 146397, 226289, 246103, 249526, 264307,
+ * 349038
*******************************************************************************/
package org.eclipse.core.databinding.observable.map;
@@ -60,7 +61,7 @@
private final Realm realm;
private PrivateChangeSupport changeSupport;
- private boolean disposed = false;
+ private volatile boolean disposed = false;
private boolean stale;
@@ -93,23 +94,27 @@
}
public synchronized void addMapChangeListener(IMapChangeListener listener) {
- if (!disposed)
+ if (!disposed) {
changeSupport.addListener(MapChangeEvent.TYPE, listener);
+ }
}
public synchronized void removeMapChangeListener(IMapChangeListener listener) {
- if (!disposed)
+ if (!disposed) {
changeSupport.removeListener(MapChangeEvent.TYPE, listener);
+ }
}
public synchronized void addChangeListener(IChangeListener listener) {
- if (!disposed)
+ if (!disposed) {
changeSupport.addChangeListener(listener);
+ }
}
public synchronized void addStaleListener(IStaleListener listener) {
- if (!disposed)
+ if (!disposed) {
changeSupport.addStaleListener(listener);
+ }
}
/**
@@ -123,17 +128,19 @@
/**
* @since 1.2
*/
- public void addDisposeListener(IDisposeListener listener) {
- if (!disposed)
+ public synchronized void addDisposeListener(IDisposeListener listener) {
+ if (!disposed) {
changeSupport.addDisposeListener(listener);
+ }
}
/**
* @since 1.2
*/
- public void removeDisposeListener(IDisposeListener listener) {
- if (!disposed)
+ public synchronized void removeDisposeListener(IDisposeListener listener) {
+ if (!disposed) {
changeSupport.removeDisposeListener(listener);
+ }
}
/**
@@ -176,11 +183,15 @@
}
public synchronized void removeChangeListener(IChangeListener listener) {
- changeSupport.removeChangeListener(listener);
+ if (!disposed) {
+ changeSupport.removeChangeListener(listener);
+ }
}
public synchronized void removeStaleListener(IStaleListener listener) {
- changeSupport.removeStaleListener(listener);
+ if (!disposed) {
+ changeSupport.removeStaleListener(listener);
+ }
}
/**
diff --git a/tests/org.eclipse.jface.tests.databinding/src/org/eclipse/core/tests/databinding/observable/list/AbstractObservableListTest.java b/tests/org.eclipse.jface.tests.databinding/src/org/eclipse/core/tests/databinding/observable/list/AbstractObservableListTest.java
index fde28d4..5bea23e 100755
--- a/tests/org.eclipse.jface.tests.databinding/src/org/eclipse/core/tests/databinding/observable/list/AbstractObservableListTest.java
+++ b/tests/org.eclipse.jface.tests.databinding/src/org/eclipse/core/tests/databinding/observable/list/AbstractObservableListTest.java
@@ -8,7 +8,7 @@
* Contributors:
* Brad Reynolds - initial API and implementation
* Brad Reynolds - bug 167204
- * Matthew Hall - bugs 208858, 213145, 247367
+ * Matthew Hall - bugs 208858, 213145, 247367, 349038
******************************************************************************/
package org.eclipse.core.tests.databinding.observable.list;
@@ -23,7 +23,9 @@
import org.eclipse.core.databinding.observable.ChangeEvent;
import org.eclipse.core.databinding.observable.Diffs;
+import org.eclipse.core.databinding.observable.DisposeEvent;
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.IObservableCollection;
import org.eclipse.core.databinding.observable.IStaleListener;
@@ -174,6 +176,29 @@
});
}
+ public void testAddDisposeListener_AfterDispose() {
+ list.dispose();
+ list.addDisposeListener(new IDisposeListener() {
+ public void handleDispose(DisposeEvent event) {
+ // do nothing
+ }
+ });
+ }
+
+ public void testRemoveDisposeListener_AfterDispose() {
+ list.dispose();
+ list.removeDisposeListener(new IDisposeListener() {
+ public void handleDispose(DisposeEvent event) {
+ // do nothing
+ }
+ });
+ }
+
+ public void testHasListeners_AfterDispose() {
+ list.dispose();
+ list.hasListeners();
+ }
+
public static Test suite() {
TestSuite suite = new TestSuite(AbstractObservableListTest.class.getName());
suite.addTestSuite(AbstractObservableListTest.class);
@@ -245,6 +270,10 @@
protected void fireListChange(ListDiff diff) {
super.fireListChange(diff);
}
+
+ protected synchronized boolean hasListeners() {
+ return super.hasListeners();
+ }
}
static class MutableObservableListStub extends AbstractObservableListStub {
diff --git a/tests/org.eclipse.jface.tests.databinding/src/org/eclipse/core/tests/databinding/observable/map/AbstractObservableMapTest.java b/tests/org.eclipse.jface.tests.databinding/src/org/eclipse/core/tests/databinding/observable/map/AbstractObservableMapTest.java
index 7f617e8..32c1d0f 100644
--- a/tests/org.eclipse.jface.tests.databinding/src/org/eclipse/core/tests/databinding/observable/map/AbstractObservableMapTest.java
+++ b/tests/org.eclipse.jface.tests.databinding/src/org/eclipse/core/tests/databinding/observable/map/AbstractObservableMapTest.java
@@ -7,6 +7,7 @@
*
* Contributors:
* Brad Reynolds - initial API and implementation
+ * Matthew Hall - bug 349038
******************************************************************************/
package org.eclipse.core.tests.databinding.observable.map;
@@ -15,7 +16,15 @@
import junit.framework.TestCase;
+import org.eclipse.core.databinding.observable.ChangeEvent;
+import org.eclipse.core.databinding.observable.DisposeEvent;
+import org.eclipse.core.databinding.observable.IChangeListener;
+import org.eclipse.core.databinding.observable.IDisposeListener;
+import org.eclipse.core.databinding.observable.IStaleListener;
+import org.eclipse.core.databinding.observable.StaleEvent;
import org.eclipse.core.databinding.observable.map.AbstractObservableMap;
+import org.eclipse.core.databinding.observable.map.IMapChangeListener;
+import org.eclipse.core.databinding.observable.map.MapChangeEvent;
import org.eclipse.core.databinding.observable.map.MapDiff;
import org.eclipse.jface.databinding.conformance.util.CurrentRealm;
import org.eclipse.jface.databinding.conformance.util.RealmTester;
@@ -74,7 +83,84 @@
}
});
}
-
+
+ public void testAddListChangeListener_AfterDispose() {
+ map.dispose();
+ map.addMapChangeListener(new IMapChangeListener() {
+ public void handleMapChange(MapChangeEvent event) {
+ // do nothing
+ }
+ });
+ }
+
+ public void testRemoveListChangeListener_AfterDispose() {
+ map.dispose();
+ map.removeMapChangeListener(new IMapChangeListener() {
+ public void handleMapChange(MapChangeEvent event) {
+ // do nothing
+ }
+ });
+ }
+
+ public void testAddChangeListener_AfterDispose() {
+ map.dispose();
+ map.addChangeListener(new IChangeListener() {
+ public void handleChange(ChangeEvent event) {
+ // do nothing
+ }
+ });
+ }
+
+ public void testRemoveChangeListener_AfterDispose() {
+ map.dispose();
+ map.removeChangeListener(new IChangeListener() {
+ public void handleChange(ChangeEvent event) {
+ // do nothing
+ }
+ });
+ }
+
+ public void testAddStaleListener_AfterDispose() {
+ map.dispose();
+ map.addStaleListener(new IStaleListener() {
+ public void handleStale(StaleEvent staleEvent) {
+ // do nothing
+ }
+ });
+ }
+
+ public void testRemoveStaleListener_AfterDispose() {
+ map.dispose();
+ map.removeStaleListener(new IStaleListener() {
+ public void handleStale(StaleEvent staleEvent) {
+ // do nothing
+ }
+ });
+ }
+
+ public void testAddDisposeListener_AfterDispose() {
+ map.dispose();
+ map.addDisposeListener(new IDisposeListener() {
+ public void handleDispose(DisposeEvent event) {
+ // do nothing
+ }
+ });
+ }
+
+ public void testRemoveDisposeListener_AfterDispose() {
+ map.dispose();
+ map.removeDisposeListener(new IDisposeListener() {
+ public void handleDispose(DisposeEvent event) {
+ // do nothing
+ }
+ });
+ }
+
+ public void testHasListeners_AfterDispose() {
+ map.dispose();
+ map.hasListeners();
+ }
+
static class AbstractObservableMapStub extends AbstractObservableMap {
public Set entrySet() {
return null;
@@ -91,5 +177,9 @@
protected void fireStale() {
super.fireStale();
}
+
+ protected synchronized boolean hasListeners() {
+ return super.hasListeners();
+ }
}
}
