Skip to main content
aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorThomas Watson2019-05-29 10:13:36 -0400
committerThomas Watson2019-06-10 08:19:27 -0400
commit6a09934020229233b209d32545fbcb0a0bdd1128 (patch)
tree366f2506b251f0c0c4ba07533362bf31b094ffeb
parentfe505109e562280fc0582b2e450645c320bec8b4 (diff)
downloadrt.equinox.framework-6a09934020229233b209d32545fbcb0a0bdd1128.tar.gz
rt.equinox.framework-6a09934020229233b209d32545fbcb0a0bdd1128.tar.xz
rt.equinox.framework-6a09934020229233b209d32545fbcb0a0bdd1128.zip
Bug 547830 - fix possible deadlock in EnhancedExecutor.await()
The count used in EnhancedExecutor is dangerous because it increments before dispatching work to the executor. If any exception happens while dispatching the task then the count will not be decremented. This changes the code to use futures instead to see when the work is done. Change-Id: Ie042d0c269cffa95dc61d917f0c1a873ba448294 Signed-off-by: Thomas Watson <tjwatson@us.ibm.com>
-rw-r--r--bundles/org.eclipse.osgi.tests/META-INF/MANIFEST.MF2
-rw-r--r--bundles/org.eclipse.osgi.tests/pom.xml2
-rw-r--r--bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/container/TestModuleContainer.java2
-rw-r--r--bundles/org.eclipse.osgi/META-INF/MANIFEST.MF2
-rwxr-xr-xbundles/org.eclipse.osgi/felix/src/org/apache/felix/resolver/ResolverImpl.java78
-rw-r--r--bundles/org.eclipse.osgi/pom.xml2
6 files changed, 45 insertions, 43 deletions
diff --git a/bundles/org.eclipse.osgi.tests/META-INF/MANIFEST.MF b/bundles/org.eclipse.osgi.tests/META-INF/MANIFEST.MF
index 58c45dd4e..2524696c8 100644
--- a/bundles/org.eclipse.osgi.tests/META-INF/MANIFEST.MF
+++ b/bundles/org.eclipse.osgi.tests/META-INF/MANIFEST.MF
@@ -2,7 +2,7 @@ Manifest-Version: 1.0
Bundle-ManifestVersion: 2
Bundle-Name: Core OSGi Tests
Bundle-SymbolicName: org.eclipse.osgi.tests;singleton:=true
-Bundle-Version: 3.13.500.qualifier
+Bundle-Version: 3.13.600.qualifier
Bundle-Vendor: Eclipse.org
Bundle-Localization: plugin
Require-Bundle:
diff --git a/bundles/org.eclipse.osgi.tests/pom.xml b/bundles/org.eclipse.osgi.tests/pom.xml
index 20e87650f..8c07556e5 100644
--- a/bundles/org.eclipse.osgi.tests/pom.xml
+++ b/bundles/org.eclipse.osgi.tests/pom.xml
@@ -19,7 +19,7 @@
</parent>
<groupId>org.eclipse.osgi</groupId>
<artifactId>org.eclipse.osgi.tests</artifactId>
- <version>3.13.500-SNAPSHOT</version>
+ <version>3.13.600-SNAPSHOT</version>
<packaging>eclipse-test-plugin</packaging>
<properties>
diff --git a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/container/TestModuleContainer.java b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/container/TestModuleContainer.java
index a2900f742..3e83af0f1 100644
--- a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/container/TestModuleContainer.java
+++ b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/container/TestModuleContainer.java
@@ -1832,7 +1832,7 @@ public class TestModuleContainer extends AbstractTest {
Assert.assertEquals("n3 should resolve.", State.RESOLVED, uses_n3.getState());
}
- // DISABLE see bug 498064 @Test
+ @Test
public void testUsesTimeout() throws BundleException {
// Always want to go to zero threads when idle
int coreThreads = 0;
diff --git a/bundles/org.eclipse.osgi/META-INF/MANIFEST.MF b/bundles/org.eclipse.osgi/META-INF/MANIFEST.MF
index 1f3e297a4..fc081bfdb 100644
--- a/bundles/org.eclipse.osgi/META-INF/MANIFEST.MF
+++ b/bundles/org.eclipse.osgi/META-INF/MANIFEST.MF
@@ -101,7 +101,7 @@ Bundle-Activator: org.eclipse.osgi.internal.framework.SystemBundleActivator
Bundle-Description: %systemBundle
Bundle-Copyright: %copyright
Bundle-Vendor: %eclipse.org
-Bundle-Version: 3.14.0.qualifier
+Bundle-Version: 3.14.100.qualifier
Bundle-Localization: systembundle
Bundle-DocUrl: http://www.eclipse.org
Eclipse-ExtensibleAPI: true
diff --git a/bundles/org.eclipse.osgi/felix/src/org/apache/felix/resolver/ResolverImpl.java b/bundles/org.eclipse.osgi/felix/src/org/apache/felix/resolver/ResolverImpl.java
index 43975c7bf..3ec67c4c5 100755
--- a/bundles/org.eclipse.osgi/felix/src/org/apache/felix/resolver/ResolverImpl.java
+++ b/bundles/org.eclipse.osgi/felix/src/org/apache/felix/resolver/ResolverImpl.java
@@ -2448,8 +2448,8 @@ public class ResolverImpl implements Resolver
private static class EnhancedExecutor
{
private final Executor executor;
- private final AtomicInteger count = new AtomicInteger();
- private Throwable throwable;
+ private final Queue<Future<Void>> awaiting = new ConcurrentLinkedQueue<>();
+ private final AtomicReference<Throwable> throwable = new AtomicReference<>();
public EnhancedExecutor(Executor executor)
{
@@ -2458,8 +2458,7 @@ public class ResolverImpl implements Resolver
public void execute(final Runnable runnable)
{
- count.incrementAndGet();
- executor.execute(new Runnable()
+ FutureTask<Void> task = new FutureTask(new Runnable()
{
public void run()
{
@@ -2469,59 +2468,62 @@ public class ResolverImpl implements Resolver
}
catch (Throwable t)
{
- synchronized (count)
- {
- if (throwable == null)
- {
- throwable = t;
- }
- }
- }
- finally
- {
- if (count.decrementAndGet() == 0)
- {
- synchronized (count)
- {
- count.notifyAll();
- }
- }
+ throwable.compareAndSet(null, t);
}
}
- });
+ }, (Void) null);
+ // must have a happens-first to add the task to awaiting
+ awaiting.add(task);
+ try
+ {
+ executor.execute(task);
+ }
+ catch (Throwable t)
+ {
+ // if the task did not get added successfully to the executor we must cancel
+ // the task so we don't await on it
+ task.cancel(false);
+ throwable.compareAndSet(null, t);
+ }
}
public void await()
{
- synchronized (count)
+ Future<Void> awaitTask;
+ while (throwable.get() == null && (awaitTask = awaiting.poll()) != null)
{
- if (count.get() > 0)
+ if (!awaitTask.isDone() && !awaitTask.isCancelled())
{
try
{
- count.wait();
+ awaitTask.get();
+ }
+ catch (CancellationException e)
+ {
+ // ignore; will have throwable set
}
catch (InterruptedException e)
{
throw new IllegalStateException(e);
}
- if (throwable != null)
+ catch (ExecutionException e)
{
- if (throwable instanceof RuntimeException)
- {
- throw (RuntimeException) throwable;
- }
- else if (throwable instanceof Error)
- {
- throw (Error) throwable;
- }
- else
- {
- throw new RuntimeException(throwable);
- }
+ throw new RuntimeException(e.getCause());
}
}
}
+ Throwable t = throwable.get();
+ if (t!= null) {
+ if (t instanceof Runnable)
+ {
+ throw (RuntimeException) t;
+ }
+ else if (t instanceof Error)
+ {
+ throw (Error) t;
+ }
+ throw new RuntimeException(t);
+ }
}
}
diff --git a/bundles/org.eclipse.osgi/pom.xml b/bundles/org.eclipse.osgi/pom.xml
index dd81d86ba..9ae03a65b 100644
--- a/bundles/org.eclipse.osgi/pom.xml
+++ b/bundles/org.eclipse.osgi/pom.xml
@@ -19,7 +19,7 @@
</parent>
<groupId>org.eclipse.osgi</groupId>
<artifactId>org.eclipse.osgi</artifactId>
- <version>3.14.0-SNAPSHOT</version>
+ <version>3.14.100-SNAPSHOT</version>
<packaging>eclipse-plugin</packaging>
<build>

Back to the top