diff options
author | Jonah Graham | 2015-12-24 14:53:56 +0000 |
---|---|---|
committer | Sarika Sinha | 2016-01-04 05:28:19 +0000 |
commit | 535e4f96bbdf4e793f22d99b2e3fb9fe17267438 (patch) | |
tree | 7473dbe25accc18d672333512d924c7c73eb7676 | |
parent | d91efcae543057148210eb78a776790859f635c2 (diff) | |
download | eclipse.platform.debug-I20160112-1800.tar.gz eclipse.platform.debug-I20160112-1800.tar.xz eclipse.platform.debug-I20160112-1800.zip |
Bug 484882: Synchronize access to fSortedConfigNamesI20160112-1800I20160112-0800I20160105-1000I20160105-0800
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>
-rw-r--r-- | org.eclipse.debug.core/core/org/eclipse/debug/internal/core/LaunchManager.java | 4 | ||||
-rw-r--r-- | org.eclipse.debug.tests/src/org/eclipse/debug/tests/launching/LaunchManagerTests.java | 55 |
2 files changed, 57 insertions, 2 deletions
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 7ecf979fe..4973e3f52 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 @@ public class LaunchManager extends PlatformObject implements ILaunchManager, IRe /** * 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 @@ public class LaunchManager extends PlatformObject implements ILaunchManager, IRe /** * 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 7cdb3eab5..0b6891499 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 @@ public class LaunchManagerTests extends AbstractLaunchTest { 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$ + } + } + } } |