This is an automated email from the ASF dual-hosted git repository. heneveld pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/brooklyn-server.git
commit c70d9ce56220b3f21204981de10f949f1fb7dec2 Author: Alex Heneveld <[email protected]> AuthorDate: Mon May 15 10:10:42 2023 +0100 fix rare bug in mutex code it could happen that one acquirer gets a view of a semaphore before it registers its usage, meanwhile another removes it --- .../org/apache/brooklyn/util/core/mutex/MutexSupport.java | 5 ++--- .../brooklyn/util/core/mutex/SemaphoreWithOwners.java | 15 +++++++-------- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/core/src/main/java/org/apache/brooklyn/util/core/mutex/MutexSupport.java b/core/src/main/java/org/apache/brooklyn/util/core/mutex/MutexSupport.java index aadff08bb5..92e30b16d9 100644 --- a/core/src/main/java/org/apache/brooklyn/util/core/mutex/MutexSupport.java +++ b/core/src/main/java/org/apache/brooklyn/util/core/mutex/MutexSupport.java @@ -102,10 +102,9 @@ public class MutexSupport implements WithMutexes { @Override public synchronized void releaseMutex(String mutexId) { - SemaphoreWithOwners s; if (log.isDebugEnabled()) log.debug("Releasing mutex: "+mutexId+"@"+this); - synchronized (this) { s = semaphores.get(mutexId); } + SemaphoreWithOwners s = semaphores.get(mutexId); if (s==null) throw new IllegalStateException("No mutex known for '"+mutexId+"'"); s.release(); cleanupMutex(mutexId); @@ -121,6 +120,6 @@ public class MutexSupport implements WithMutexes { * from manipulating the semaphore objects themselves. */ public synchronized Map<String,SemaphoreWithOwners> getAllSemaphores() { - return ImmutableMap.<String,SemaphoreWithOwners>copyOf(semaphores); + return ImmutableMap.copyOf(semaphores); } } \ No newline at end of file diff --git a/core/src/main/java/org/apache/brooklyn/util/core/mutex/SemaphoreWithOwners.java b/core/src/main/java/org/apache/brooklyn/util/core/mutex/SemaphoreWithOwners.java index 0a1ab9b8f2..2182306f8d 100644 --- a/core/src/main/java/org/apache/brooklyn/util/core/mutex/SemaphoreWithOwners.java +++ b/core/src/main/java/org/apache/brooklyn/util/core/mutex/SemaphoreWithOwners.java @@ -18,17 +18,16 @@ */ package org.apache.brooklyn.util.core.mutex; +import com.google.common.collect.ImmutableList; +import org.apache.brooklyn.util.collections.MutableSet; +import org.apache.brooklyn.util.exceptions.Exceptions; + import java.util.ArrayList; -import java.util.LinkedHashSet; import java.util.List; import java.util.Set; import java.util.concurrent.Semaphore; import java.util.concurrent.TimeUnit; -import org.apache.brooklyn.util.exceptions.Exceptions; - -import com.google.common.collect.ImmutableList; - /** a subclass of {@link Semaphore} * which tracks who created and released the semaphores, * and which requires the same thread to release as created it. */ @@ -42,7 +41,7 @@ public class SemaphoreWithOwners extends Semaphore { } private static final long serialVersionUID = -5303474637353009454L; final private List<Thread> owningThreads = new ArrayList<Thread>(); - final private Set<Thread> requestingThreads = new LinkedHashSet<Thread>(); + final private Set<Thread> requestingThreads = MutableSet.of(); @Override public void acquire() throws InterruptedException { @@ -188,7 +187,7 @@ public class SemaphoreWithOwners extends Semaphore { /** true iff there are any owners or any requesters (callers blocked trying to acquire) */ public synchronized boolean isInUse() { - return !owningThreads.isEmpty() || !requestingThreads.isEmpty(); + return !requestingThreads.isEmpty() || !owningThreads.isEmpty(); } /** true iff the calling thread is one of the owners */ @@ -224,7 +223,7 @@ public class SemaphoreWithOwners extends Semaphore { /** Indicate that the calling thread is going to acquire or tryAcquire, * in order to set up the semaphore's isInUse() value appropriately for certain checks. * It *must* do so after invoking this method. */ - public void indicateCallingThreadWillRequest() { + public synchronized void indicateCallingThreadWillRequest() { requestingThreads.add(Thread.currentThread()); }
