diff --git a/bundles/org.eclipse.equinox.ds.tests/.gitignore b/bundles/org.eclipse.equinox.ds.tests/.gitignore index f02e6a763a3..1de704a8184 100644 --- a/bundles/org.eclipse.equinox.ds.tests/.gitignore +++ b/bundles/org.eclipse.equinox.ds.tests/.gitignore @@ -1 +1,2 @@ /scr_test +/OSGI-INF/org.eclipse.*.xml diff --git a/bundles/org.eclipse.equinox.ds.tests/OSGI-INF/org.eclipse.equinox.ds.tests.LazyServiceComponentActivationDeadLockTest$Component1.xml b/bundles/org.eclipse.equinox.ds.tests/OSGI-INF/org.eclipse.equinox.ds.tests.LazyServiceComponentActivationDeadLockTest$Component1.xml deleted file mode 100644 index db7e6742717..00000000000 --- a/bundles/org.eclipse.equinox.ds.tests/OSGI-INF/org.eclipse.equinox.ds.tests.LazyServiceComponentActivationDeadLockTest$Component1.xml +++ /dev/null @@ -1,9 +0,0 @@ - - - - - - - - - \ No newline at end of file diff --git a/bundles/org.eclipse.equinox.ds.tests/OSGI-INF/org.eclipse.equinox.ds.tests.LazyServiceComponentActivationDeadLockTest$Component2.xml b/bundles/org.eclipse.equinox.ds.tests/OSGI-INF/org.eclipse.equinox.ds.tests.LazyServiceComponentActivationDeadLockTest$Component2.xml deleted file mode 100644 index 068f8337ffd..00000000000 --- a/bundles/org.eclipse.equinox.ds.tests/OSGI-INF/org.eclipse.equinox.ds.tests.LazyServiceComponentActivationDeadLockTest$Component2.xml +++ /dev/null @@ -1,8 +0,0 @@ - - - - - - - - \ No newline at end of file diff --git a/bundles/org.eclipse.equinox.ds.tests/OSGI-INF/org.eclipse.equinox.ds.tests.LazyServiceComponentActivationDeadLockTest$Component3.xml b/bundles/org.eclipse.equinox.ds.tests/OSGI-INF/org.eclipse.equinox.ds.tests.LazyServiceComponentActivationDeadLockTest$Component3.xml deleted file mode 100644 index 4eeb19697bf..00000000000 --- a/bundles/org.eclipse.equinox.ds.tests/OSGI-INF/org.eclipse.equinox.ds.tests.LazyServiceComponentActivationDeadLockTest$Component3.xml +++ /dev/null @@ -1,9 +0,0 @@ - - - - - - - - - \ No newline at end of file diff --git a/bundles/org.eclipse.equinox.ds.tests/OSGI-INF/org.eclipse.equinox.ds.tests.LazyServiceComponentActivationDeadLockTest$Component4$AwaitComponent5Activation.xml b/bundles/org.eclipse.equinox.ds.tests/OSGI-INF/org.eclipse.equinox.ds.tests.LazyServiceComponentActivationDeadLockTest$Component4$AwaitComponent5Activation.xml deleted file mode 100644 index 22ddecc564b..00000000000 --- a/bundles/org.eclipse.equinox.ds.tests/OSGI-INF/org.eclipse.equinox.ds.tests.LazyServiceComponentActivationDeadLockTest$Component4$AwaitComponent5Activation.xml +++ /dev/null @@ -1,7 +0,0 @@ - - - - - - - \ No newline at end of file diff --git a/bundles/org.eclipse.equinox.ds.tests/OSGI-INF/org.eclipse.equinox.ds.tests.LazyServiceComponentActivationDeadLockTest$Component4.xml b/bundles/org.eclipse.equinox.ds.tests/OSGI-INF/org.eclipse.equinox.ds.tests.LazyServiceComponentActivationDeadLockTest$Component4.xml deleted file mode 100644 index 0a23ac7b007..00000000000 --- a/bundles/org.eclipse.equinox.ds.tests/OSGI-INF/org.eclipse.equinox.ds.tests.LazyServiceComponentActivationDeadLockTest$Component4.xml +++ /dev/null @@ -1,9 +0,0 @@ - - - - - - - - - \ No newline at end of file diff --git a/bundles/org.eclipse.equinox.ds.tests/OSGI-INF/org.eclipse.equinox.ds.tests.LazyServiceComponentActivationDeadLockTest$Component5$ActivationStartedIndicator.xml b/bundles/org.eclipse.equinox.ds.tests/OSGI-INF/org.eclipse.equinox.ds.tests.LazyServiceComponentActivationDeadLockTest$Component5$ActivationStartedIndicator.xml deleted file mode 100644 index 18fc4d5a049..00000000000 --- a/bundles/org.eclipse.equinox.ds.tests/OSGI-INF/org.eclipse.equinox.ds.tests.LazyServiceComponentActivationDeadLockTest$Component5$ActivationStartedIndicator.xml +++ /dev/null @@ -1,7 +0,0 @@ - - - - - - - \ No newline at end of file diff --git a/bundles/org.eclipse.equinox.ds.tests/OSGI-INF/org.eclipse.equinox.ds.tests.LazyServiceComponentActivationDeadLockTest$Component5.xml b/bundles/org.eclipse.equinox.ds.tests/OSGI-INF/org.eclipse.equinox.ds.tests.LazyServiceComponentActivationDeadLockTest$Component5.xml deleted file mode 100644 index d0f6baadb80..00000000000 --- a/bundles/org.eclipse.equinox.ds.tests/OSGI-INF/org.eclipse.equinox.ds.tests.LazyServiceComponentActivationDeadLockTest$Component5.xml +++ /dev/null @@ -1,9 +0,0 @@ - - - - - - - - - \ No newline at end of file diff --git a/bundles/org.eclipse.equinox.ds.tests/src/org/eclipse/equinox/ds/tests/LazyServiceComponentActivationDeadLockTest.java b/bundles/org.eclipse.equinox.ds.tests/src/org/eclipse/equinox/ds/tests/LazyServiceComponentActivationDeadLockTest.java index 91c12ee3333..0ff628f7310 100644 --- a/bundles/org.eclipse.equinox.ds.tests/src/org/eclipse/equinox/ds/tests/LazyServiceComponentActivationDeadLockTest.java +++ b/bundles/org.eclipse.equinox.ds.tests/src/org/eclipse/equinox/ds/tests/LazyServiceComponentActivationDeadLockTest.java @@ -14,9 +14,9 @@ package org.eclipse.equinox.ds.tests; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; import java.util.ArrayList; import java.util.List; @@ -60,13 +60,6 @@ public T getService(Bundle bundle, ServiceRegistration registration) { @Override public void ungetService(Bundle bundle, ServiceRegistration registration, T service) { } - - } - - class Service1 { - } - - class Service2 { } CountDownLatch l1 = new CountDownLatch(1); @@ -75,13 +68,13 @@ class Service2 { ServiceFactory factory1 = new SimpleServiceFactory<>((bundle, registration) -> { countDownAndAwaitOther(l1, l2); - ctx.getService(ctx.getServiceReference(Service2.class)); - return new Service1(); + Service2 s = ctx.getService(ctx.getServiceReference(Service2.class)); + return new Service1(s); }); ServiceFactory factory2 = new SimpleServiceFactory<>((bundle, registration) -> { countDownAndAwaitOther(l2, l1); - ctx.getService(ctx.getServiceReference(Service1.class)); - return new Service2(); + Service1 s = ctx.getService(ctx.getServiceReference(Service1.class)); + return new Service2(s); }); ctx.registerService(Service1.class, factory1, null); ctx.registerService(Service2.class, factory2, null); @@ -92,16 +85,31 @@ class Service2 { Future service2 = executor.submit(() -> ctx.getService(ctx.getServiceReference(Service2.class))); Service1 s1 = service1.get(5, TimeUnit.SECONDS); // times out in case of dead-lock Service2 s2 = service2.get(5, TimeUnit.SECONDS); // times out in case of dead-lock - if (s1 != null && s2 != null) { - fail("At least one dead-locked component should be null"); - } - // TODO: when we have a strategy to re-cover from the deadlock, check that both - // services are not null. + assertNotNull(s1); + assertNotNull(s2); + assertFalse(s1.s2 == null && s2.s1 == null); + assertTrue(s1.s2 == null || s2.s1 == null); } finally { executor.shutdown(); } } + class Service1 { + final Service2 s2; + + Service1(Service2 s2) { + this.s2 = s2; + } + } + + class Service2 { + final Service1 s1; + + Service2(Service1 s1) { + this.s1 = s1; + } + } + void countDownAndAwaitOther(CountDownLatch l1, CountDownLatch l2) { l1.countDown(); try { @@ -122,16 +130,12 @@ public void testLateBindingInSameBundleDeadLock() throws Exception { // component might or might not be null depending on in which thread the // DeadLock was detected and the SCR-ServiceFactory therefore threw an // Exception. - if (c1 != null) { - // If the main-thread did not detect the dead-lock and therefore Component1 is - // successfully created, the multi-ref from its Comp2 to Comp3 must be empty and - // its Comp4 must have its Comp5 successfully created. - assertTrue(c1.a.c3s.isEmpty()); - assertNotNull(c1.b.b); - } - // TODO: when we have a strategy to re-cover from the deadlock, check that c1 is - // never null and that c1.a.c3s is not empty and its only element has a Comp2 - // and Comp5 + assertNotNull(c1); + assertFalse(c1.a.c3s.isEmpty()); + assertNotNull(c1.b.b); + Component3 c3 = c1.a.c3s.get(0); + assertNotNull(c3.a); + assertNotNull(c3.b); } finally { executor.shutdown(); // ctx.ungetService(reference); diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/serviceregistry/ServiceFactoryUse.java b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/serviceregistry/ServiceFactoryUse.java index 45055c740f5..3a318336c0b 100644 --- a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/serviceregistry/ServiceFactoryUse.java +++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/serviceregistry/ServiceFactoryUse.java @@ -219,6 +219,9 @@ public S run() { } }); } catch (Throwable t) { + if (t instanceof ServiceException && ((ServiceException) t).getType() == ServiceUse.DEADLOCK) { + throw t; + } if (debug.DEBUG_SERVICES) { Debug.println(factory + ".getService() exception: " + t.getMessage()); //$NON-NLS-1$ Debug.printStackTrace(t); diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/serviceregistry/ServiceRegistrationImpl.java b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/serviceregistry/ServiceRegistrationImpl.java index f67041c6333..2fd3140b41c 100644 --- a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/serviceregistry/ServiceRegistrationImpl.java +++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/serviceregistry/ServiceRegistrationImpl.java @@ -14,10 +14,15 @@ package org.eclipse.osgi.internal.serviceregistry; +import java.lang.management.ManagementFactory; +import java.lang.management.ThreadInfo; +import java.lang.management.ThreadMXBean; import java.util.ArrayList; +import java.util.Arrays; import java.util.Dictionary; import java.util.List; import java.util.Map; +import java.util.Random; import java.util.concurrent.ConcurrentMap; import java.util.concurrent.locks.ReentrantLock; import org.eclipse.osgi.internal.debug.Debug; @@ -493,6 +498,8 @@ public Bundle getRegisteringBundle() { return bundle; } + private static final Random DEADLOCK_RETRY_DELAY = new Random(); + /** * Get a service object for the using BundleContext. * @@ -514,6 +521,7 @@ S getService(BundleContextImpl user, ServiceConsumer consumer) { + ")"); //$NON-NLS-1$ } /* Use a while loop to support retry if a call to a ServiceFactory fails */ + int deadLockRetry = 0; while (true) { final ServiceUse use; final boolean added; @@ -579,10 +587,37 @@ S getService(BundleContextImpl user, ServiceConsumer consumer) { } } return serviceObject; + } catch (ServiceException e) { + if ((e.getType() != ServiceUse.DEADLOCK || !isFirstCall()) && deadLockRetry < 10) { + throw e; + } + if (registry.debug.DEBUG_SERVICES) { + Debug.println("[" + Thread.currentThread().getName() + "] getServiceRetryAfterDeadlock[" //$NON-NLS-1$ //$NON-NLS-2$ + + user.getBundleImpl() + "](" + this //$NON-NLS-1$ + + ")"); //$NON-NLS-1$ + } + try { + // Try again and again with some random delay to change the timings + Thread.sleep(DEADLOCK_RETRY_DELAY.nextInt(1000) * deadLockRetry++); + } catch (InterruptedException e1) { // ignore + Thread.currentThread().interrupt(); + } } } } + private static boolean isFirstCall() { + ThreadMXBean threadMXBean = ManagementFactory.getThreadMXBean(); + ThreadInfo threadInfo = threadMXBean.getThreadInfo(Thread.currentThread().getId(), Integer.MAX_VALUE); + StackTraceElement[] stackTrace = threadInfo.getStackTrace(); + int callerFrameIndex = 4; + return Arrays.stream(stackTrace).skip(callerFrameIndex + 1L) + .noneMatch(f -> isSameMethod(f, stackTrace[callerFrameIndex])); + } + + private static boolean isSameMethod(StackTraceElement f1, StackTraceElement f2) { + return f1.getClassName().equals(f2.getClassName()) && f1.getMethodName().equals(f2.getMethodName()); + } /** * Create a new ServiceObjects for the requesting bundle.