Bug 484882: Synchronize access to fSortedConfigNames
The new test demonstrates the problem by running two threads,
one creating/deleting launch configs, the other one accessing
fSortedConfigNames.
Note if you set fSortedConfigNames to volatile without making
clearConfigNameCache synchronized you can see the test fails
generally very fast.
Change-Id: I981d7054d51e263f5d097096ffe23cdc1fa74256
Signed-off-by: Jonah Graham <jonah@kichwacoders.com>
diff --git a/org.eclipse.debug.core/core/org/eclipse/debug/internal/core/LaunchManager.java b/org.eclipse.debug.core/core/org/eclipse/debug/internal/core/LaunchManager.java
index 7ecf979..4973e3f 100644
--- a/org.eclipse.debug.core/core/org/eclipse/debug/internal/core/LaunchManager.java
+++ b/org.eclipse.debug.core/core/org/eclipse/debug/internal/core/LaunchManager.java
@@ -589,7 +589,7 @@
/**
* A cache of launch configuration names currently in the workspace.
*/
- private String[] fSortedConfigNames = null;
+ private volatile String[] fSortedConfigNames = null;
/**
* Collection of all launch configurations in the workspace.
@@ -916,7 +916,7 @@
/**
* The launch config name cache is cleared when a config is added, deleted or changed.
*/
- protected void clearConfigNameCache() {
+ protected synchronized void clearConfigNameCache() {
fSortedConfigNames = null;
}
diff --git a/org.eclipse.debug.tests/src/org/eclipse/debug/tests/launching/LaunchManagerTests.java b/org.eclipse.debug.tests/src/org/eclipse/debug/tests/launching/LaunchManagerTests.java
index 7cdb3ea..0b68914 100644
--- a/org.eclipse.debug.tests/src/org/eclipse/debug/tests/launching/LaunchManagerTests.java
+++ b/org.eclipse.debug.tests/src/org/eclipse/debug/tests/launching/LaunchManagerTests.java
@@ -10,12 +10,14 @@
*******************************************************************************/
package org.eclipse.debug.tests.launching;
+import org.eclipse.core.runtime.CoreException;
import org.eclipse.core.runtime.NullProgressMonitor;
import org.eclipse.core.runtime.Platform;
import org.eclipse.debug.core.ILaunch;
import org.eclipse.debug.core.ILaunchConfiguration;
import org.eclipse.debug.core.ILaunchConfigurationType;
import org.eclipse.debug.core.ILaunchConfigurationWorkingCopy;
+import org.eclipse.debug.core.ILaunchManager;
import org.eclipse.debug.internal.core.LaunchManager;
import org.eclipse.debug.tests.launching.CancellingLaunchDelegate.CancellingLaunch;
@@ -318,4 +320,57 @@
config.delete();
}
}
+
+ /**
+ * There was a race condition in getting a unique name for a launch
+ * configuration.
+ * <p>
+ * Note, due to the nature of the bug, it is possible that running this test
+ * will not trigger the original bug. To increase the probability of hitting
+ * the NPE in the unpatched code, increase the size of config. However,
+ * increasing the number increases the runtime of the test substantially.
+ */
+ public void testNPE_Bug484882() throws Exception {
+ // In this thread continuously creates configs so that
+ // org.eclipse.debug.internal.core.LaunchManager.clearConfigNameCache()
+ // is called repeatedly. We also want to make lots of configurations so
+ // the runtime of getAllSortedConfigNames (called by
+ // isExistingLaunchConfigurationName
+ // below) is long enough to hit the race condition.
+ final boolean stop[] = new boolean[] { false };
+ final Throwable exception[] = new Throwable[] { null };
+ Thread thread = new Thread() {
+ @Override
+ public void run() {
+ ILaunchConfiguration config[] = new ILaunchConfiguration[10000];
+ try {
+ for (int i = 0; i < config.length && !stop[0]; i++) {
+ config[i] = getLaunchConfiguration("Name" + i); //$NON-NLS-1$
+ }
+ for (int i = 0; i < config.length; i++) {
+ if (config[i] != null) {
+ config[i].delete();
+ }
+ }
+ } catch (CoreException e) {
+ exception[0] = e;
+ }
+ }
+ };
+ thread.start();
+ try {
+ ILaunchManager launchManager = getLaunchManager();
+ while (thread.isAlive()) {
+ // This call generated an NPE
+ launchManager.isExistingLaunchConfigurationName("Name"); //$NON-NLS-1$
+ }
+ } finally {
+ stop[0] = true;
+ thread.join(1000);
+ assertFalse(thread.isAlive());
+ if (exception[0] != null) {
+ throw new Exception("Exception in Thread", exception[0]); //$NON-NLS-1$
+ }
+ }
+ }
}
