Skip to main content
aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorChristian W. Damus2016-11-08 17:20:44 +0000
committerChristian W. Damus2016-11-08 17:23:59 +0000
commit0c4d09523080e0976fbf7b0e7274c741a7ccaba7 (patch)
treeffbfd4cfc53c69edbbcd76a61658bd08a4f4168a
parent221e5b93547810e728c22f64008cf88e4f2a9efe (diff)
downloadorg.eclipse.papyrus-0c4d09523080e0976fbf7b0e7274c741a7ccaba7.tar.gz
org.eclipse.papyrus-0c4d09523080e0976fbf7b0e7274c741a7ccaba7.tar.xz
org.eclipse.papyrus-0c4d09523080e0976fbf7b0e7274c741a7ccaba7.zip
Bug 507241: DependentEMFLabelProvider does not support unsubscription during notification
Support addition/removal of listeners on the ForwardingEMFLabelProvider during iteration of its listeners (usually for notification broadcast). https://bugs.eclipse.org/bugs/show_bug.cgi?id=507241 Change-Id: I516e1e57ece304c81874f4fd80e7751eaa727743
-rw-r--r--plugins/infra/emf/org.eclipse.papyrus.infra.ui.emf/src/org/eclipse/papyrus/infra/ui/emf/providers/ForwardingEMFLabelProvider.java4
-rw-r--r--tests/junit/plugins/infra/emf/org.eclipse.papyrus.infra.ui.emf.tests/src/org/eclipse/papyrus/infra/ui/emf/providers/tests/DependentEMFLabelProviderTest.java45
2 files changed, 47 insertions, 2 deletions
diff --git a/plugins/infra/emf/org.eclipse.papyrus.infra.ui.emf/src/org/eclipse/papyrus/infra/ui/emf/providers/ForwardingEMFLabelProvider.java b/plugins/infra/emf/org.eclipse.papyrus.infra.ui.emf/src/org/eclipse/papyrus/infra/ui/emf/providers/ForwardingEMFLabelProvider.java
index a998f1a1097..3f6ee43944c 100644
--- a/plugins/infra/emf/org.eclipse.papyrus.infra.ui.emf/src/org/eclipse/papyrus/infra/ui/emf/providers/ForwardingEMFLabelProvider.java
+++ b/plugins/infra/emf/org.eclipse.papyrus.infra.ui.emf/src/org/eclipse/papyrus/infra/ui/emf/providers/ForwardingEMFLabelProvider.java
@@ -15,6 +15,7 @@ package org.eclipse.papyrus.infra.ui.emf.providers;
import java.util.Map;
import java.util.concurrent.ConcurrentMap;
+import java.util.concurrent.CopyOnWriteArrayList;
import org.eclipse.emf.common.notify.AdapterFactory;
import org.eclipse.emf.ecore.EObject;
@@ -41,6 +42,9 @@ public class ForwardingEMFLabelProvider extends StandardEMFLabelProvider {
public ForwardingEMFLabelProvider() {
super();
+ // I need to support re-entrant changes during notification
+ labelProviderListeners = new CopyOnWriteArrayList<>(labelProviderListeners);
+
// I am used in contexts where JFace label provider events are needed
setFireLabelUpdateNotifications(true);
}
diff --git a/tests/junit/plugins/infra/emf/org.eclipse.papyrus.infra.ui.emf.tests/src/org/eclipse/papyrus/infra/ui/emf/providers/tests/DependentEMFLabelProviderTest.java b/tests/junit/plugins/infra/emf/org.eclipse.papyrus.infra.ui.emf.tests/src/org/eclipse/papyrus/infra/ui/emf/providers/tests/DependentEMFLabelProviderTest.java
index 943414b4d24..9af5bef4482 100644
--- a/tests/junit/plugins/infra/emf/org.eclipse.papyrus.infra.ui.emf.tests/src/org/eclipse/papyrus/infra/ui/emf/providers/tests/DependentEMFLabelProviderTest.java
+++ b/tests/junit/plugins/infra/emf/org.eclipse.papyrus.infra.ui.emf.tests/src/org/eclipse/papyrus/infra/ui/emf/providers/tests/DependentEMFLabelProviderTest.java
@@ -15,6 +15,9 @@ package org.eclipse.papyrus.infra.ui.emf.providers.tests;
import static org.hamcrest.CoreMatchers.is;
import static org.junit.Assert.assertThat;
+import static org.junit.Assert.fail;
+
+import java.util.Map;
import org.eclipse.emf.ecore.EObject;
import org.eclipse.jface.viewers.ILabelProvider;
@@ -27,6 +30,8 @@ import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
+import com.google.common.collect.MapMaker;
+
/**
* Tests for the {@link DependentEMFLabelProvider} class.
*/
@@ -75,6 +80,31 @@ public class DependentEMFLabelProviderTest {
assertThat("Label provider did not notify", gotEvent[0], is(true));
}
+ /**
+ * Verify that we do not get any exception (e.g., concurrent modification) when
+ * a subscription is removed while the label provider is notifying.
+ */
+ @Test
+ public void safeSubscriptionRemovalOnDestroy_bug507241() {
+ // This listener indirectly triggers the unsubscription
+ ILabelProviderListener listener = __ -> fixture.getText(imported);
+ fixture.addListener(listener);
+
+ fixture.getText(imported); // Access it once to hook up the item adapters
+
+ // Add another listener that will be notified after the subscription listener
+ fixture.addListener(this::pass);
+
+ try {
+ // Delete the import
+ importing.getPackageImport(imported).destroy();
+ importing.unsetName();
+ } catch (Exception e) {
+ e.printStackTrace();
+ fail(String.format("Got %s during edit: %s", e.getClass().getSimpleName(), e.getMessage()));
+ }
+ }
+
//
// Test framework
//
@@ -92,9 +122,15 @@ public class DependentEMFLabelProviderTest {
importing.createPackageImport(imported);
}
+ void pass(Object __) {
+ // Pass
+ }
+
// For testing purposes, a label provider for packages that decorates the label
// with the label of one package that imports it (if any)
private class ImportedPackageLabelProvider extends DependentEMFLabelProvider {
+ private final Map<Package, Package> lastKnownImporter = new MapMaker().weakKeys().weakValues().makeMap();
+
@Override
protected String getText(EObject element) {
// Default
@@ -102,12 +138,17 @@ public class DependentEMFLabelProviderTest {
if (element instanceof Package) {
Package package_ = (Package) element;
- Package importer = getImportingPackage(package_);
- if (importer != null) {
+ Package importer = lastKnownImporter.computeIfAbsent(package_, this::getImportingPackage);
+
+ if ((importer != null) && importer.getImportedPackages().contains(package_)) {
+ // It is currently importing this package.
// Listen for changes in the dependency to notify on the dependent
subscribe(importer, package_);
result = String.format("%s (imported by %s)", result, getText(importer));
+ } else {
+ // In case we were subscribed
+ unsubscribe(importer, package_);
}
}

Back to the top