From bd0257254dee64b675e403d571c721cc1caa507f Mon Sep 17 00:00:00 2001 From: Hannes Wellmann Date: Thu, 30 Jun 2022 22:40:19 +0200 Subject: [PATCH] Add deadlock-detecting lock ReentrantLock to ServiceUse This is required for https://github.com/eclipse-equinox/equinox/issues/15 --- .../internal/serviceregistry/ServiceUse.java | 96 +++++++++++++++++++ .../messages/ExternalMessages.properties | 1 + .../eclipse/osgi/internal/messages/Msg.java | 1 + 3 files changed, 98 insertions(+) diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/serviceregistry/ServiceUse.java b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/serviceregistry/ServiceUse.java index 7575322f689..15cc09f80f7 100644 --- a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/serviceregistry/ServiceUse.java +++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/serviceregistry/ServiceUse.java @@ -14,8 +14,13 @@ package org.eclipse.osgi.internal.serviceregistry; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.locks.ReentrantLock; import org.eclipse.osgi.internal.framework.BundleContextImpl; import org.eclipse.osgi.internal.messages.Msg; +import org.eclipse.osgi.util.NLS; import org.osgi.framework.ServiceException; /** @@ -35,6 +40,18 @@ public class ServiceUse { /* @GuardedBy("this") */ private int useCount; + /** + * ReentrantLock for this service. Use the @{@link #getLock()} method to obtain + * the lock. + */ + private final ServiceUseLock lock = new ServiceUseLock(); + + /** + * Custom ServiceException type to indicate a deadlock occurred during service + * registration. + */ + public static final int DEADLOCK = 1001; + /** * Constructs a service use encapsulating the service object. * @@ -46,6 +63,62 @@ public class ServiceUse { this.registration = registration; } + /** + * The ReentrantLock for managing access to this ServiceUse. + * + * @return A ReentrantLock. + */ + ReentrantLock getLock() { + return lock; + } + + private static final ConcurrentMap AWAITED_LOCKS = new ConcurrentHashMap<>(); + + /** + * Acquires the lock of this ServiceUse. + * + * If this ServiceUse is locked by another thread then the current thread lies + * dormant until the lock has been acquired. + * + * @return The {@link AutoCloseable autoclosable} locked state of this + * ServiceUse + * @throws ServiceException if a deadlock with another ServiceUse is detected + */ + ServiceUseLock lock() { + boolean clearAwaitingLock = false; + boolean interrupted = false; + do { + try { + if (lock.tryLock(100_000_000L, TimeUnit.NANOSECONDS)) { // 100ms (but prevent conversion) + if (clearAwaitingLock) { + AWAITED_LOCKS.remove(Thread.currentThread()); + } + if (interrupted) { + Thread.currentThread().interrupt(); + } + return lock; + } + AWAITED_LOCKS.put(Thread.currentThread(), lock); + clearAwaitingLock = true; + Thread owner = lock.getOwner(); + if (owner != null) { // lock could be released in the meantime + ServiceUseLock crossingLock = AWAITED_LOCKS.get(owner); + if (crossingLock != null && crossingLock.getOwner() == Thread.currentThread()) { + throw new ServiceException(NLS.bind(Msg.SERVICE_USE_DEADLOCK, lock)); + } + } + // Not (yet) a dead-lock. Lock was regularly hold by another thread. Try again. + // Race conditions are not an issue here. A deadlock is a static situation and + // if we closely missed the other thread putting its awaited lock it will be + // noticed in the next loop-pass. + } catch (InterruptedException e) { + interrupted = true; + // Clear interrupted status and try again to lock, just like a plain + // synchronized. Re-interrupted before returning to the caller. + } + } while (true); + } + /** * Get a service's service object and increment the use count. * @@ -175,4 +248,27 @@ void decrementUse() { void resetUse() { useCount = 0; } + + /** + * ReentrantLock subclass with exposed {@link #getOwner()} that implements + * {@link AutoCloseable}. + * + * This lock is unlocked if the close method is invoked. It therefore can be + * used as resource of a try-with-resources block. + */ + static class ServiceUseLock extends ReentrantLock implements AutoCloseable { + private static final long serialVersionUID = 4281308691512232595L; + + @Override + protected Thread getOwner() { + return super.getOwner(); + } + + /** Close and unlock this lock. */ + @Override + public void close() { + unlock(); + } + + } } diff --git a/bundles/org.eclipse.osgi/supplement/src/org/eclipse/osgi/internal/messages/ExternalMessages.properties b/bundles/org.eclipse.osgi/supplement/src/org/eclipse/osgi/internal/messages/ExternalMessages.properties index 9b5cdfb45b4..d0abb02e7d6 100644 --- a/bundles/org.eclipse.osgi/supplement/src/org/eclipse/osgi/internal/messages/ExternalMessages.properties +++ b/bundles/org.eclipse.osgi/supplement/src/org/eclipse/osgi/internal/messages/ExternalMessages.properties @@ -30,6 +30,7 @@ SERVICE_OBJECTS_UNGET_ARGUMENT_EXCEPTION=The service parameter was not provided SERVICE_ALREADY_UNREGISTERED_EXCEPTION=The service has been unregistered SERVICE_EMPTY_CLASS_LIST_EXCEPTION=The array of service names is empty SERVICE_USE_OVERFLOW=The use count for the service overflowed. +SERVICE_USE_DEADLOCK=Deadlock while trying to acquire use lock {1}. HEADER_DUPLICATE_KEY_EXCEPTION=The key \"{0}\" already exists in another case variation FILTER_MISSING_LEFTPAREN=Missing ''('' at \"{0}\" FILTER_MISSING_RIGHTPAREN=Missing '')'' at \"{0}\" diff --git a/bundles/org.eclipse.osgi/supplement/src/org/eclipse/osgi/internal/messages/Msg.java b/bundles/org.eclipse.osgi/supplement/src/org/eclipse/osgi/internal/messages/Msg.java index 5434a0e4c66..03f78a39da3 100644 --- a/bundles/org.eclipse.osgi/supplement/src/org/eclipse/osgi/internal/messages/Msg.java +++ b/bundles/org.eclipse.osgi/supplement/src/org/eclipse/osgi/internal/messages/Msg.java @@ -44,6 +44,7 @@ public class Msg extends NLS { public static String SERVICE_USE_OVERFLOW; public static String SERVICE_OBJECTS_UNGET_ARGUMENT_EXCEPTION; + public static String SERVICE_USE_DEADLOCK; public static String SystemModule_LockError;