Repository: brooklyn-server Updated Branches: refs/heads/master 84e9e1986 -> fede005e1
improve locking for sensor values fixes https://issues.apache.org/jira/browse/BROOKLYN-544, introducing a public Lock for writes and callers, and the weaker uglier monitor for very short internal synchronized blocks. also adds a Locks convenience class for running code with a lock. Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/22178b65 Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/22178b65 Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/22178b65 Branch: refs/heads/master Commit: 22178b6587ca394f2da1b2f8fbd40734c691ae97 Parents: d69f3a5 Author: Alex Heneveld <[email protected]> Authored: Wed Oct 11 20:50:22 2017 +0100 Committer: Alex Heneveld <[email protected]> Committed: Wed Oct 11 20:50:22 2017 +0100 ---------------------------------------------------------------------- .../brooklyn/core/entity/AbstractEntity.java | 74 +++++++----- .../brooklyn/core/entity/EntityInternal.java | 3 +- .../mgmt/internal/LocalSubscriptionManager.java | 9 ++ .../brooklyn/core/sensor/AttributeMap.java | 117 ++++++++++++++----- .../entity/group/AbstractGroupImpl.java | 13 ++- .../brooklyn/core/entity/AttributeMapTest.java | 5 +- .../apache/brooklyn/util/concurrent/Locks.java | 47 ++++++++ .../brooklyn/util/collections/LocksTest.java | 37 ++++++ 8 files changed, 235 insertions(+), 70 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/22178b65/core/src/main/java/org/apache/brooklyn/core/entity/AbstractEntity.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/entity/AbstractEntity.java b/core/src/main/java/org/apache/brooklyn/core/entity/AbstractEntity.java index dfbbc8f..dc18675 100644 --- a/core/src/main/java/org/apache/brooklyn/core/entity/AbstractEntity.java +++ b/core/src/main/java/org/apache/brooklyn/core/entity/AbstractEntity.java @@ -26,6 +26,7 @@ import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.concurrent.locks.Lock; import org.apache.brooklyn.api.effector.Effector; import org.apache.brooklyn.api.entity.Application; @@ -96,6 +97,7 @@ import org.apache.brooklyn.core.typereg.RegisteredTypes; import org.apache.brooklyn.util.collections.MutableList; import org.apache.brooklyn.util.collections.MutableMap; import org.apache.brooklyn.util.collections.MutableSet; +import org.apache.brooklyn.util.concurrent.Locks; import org.apache.brooklyn.util.core.flags.FlagUtils; import org.apache.brooklyn.util.core.flags.TypeCoercions; import org.apache.brooklyn.util.guava.Maybe; @@ -525,13 +527,16 @@ public abstract class AbstractEntity extends AbstractBrooklynObject implements E if (displayNameAutoGenerated) displayName.set(getEntityType().getSimpleName()+":"+Strings.maxlen(getId(), 4)); } - /** Where code needs to synch on the attributes, it can access the low-level object used for synching - * through this method. Internally, all attribute updates synch on this object. Code wishing to - * update attributes or publish while holding some other lock should acquire the monitor on this - * object first to prevent deadlock. */ + /** + * Where code needs to enforce single threading, this {@link Lock} can be used. + * <p> + * Care must be taken to avoid deadlock, with canoncial orders carefully defined + * if this is used by any lockholder or any other locks used while holding this lock. + * See {@link AttributeMap#getLockInternal()} for more detail. + */ @Beta - protected Object getAttributesSynchObjectInternal() { - return attributesInternal.getSynchObjectInternal(); + protected Lock getLockInternal() { + return attributesInternal.getLockInternal(); } @Override @@ -647,7 +652,7 @@ public abstract class AbstractEntity extends AbstractBrooklynObject implements E checkNotNull(child, "child must not be null (for entity %s)", this); CatalogUtils.setCatalogItemIdOnAddition(this, child); - synchronized (getAttributesSynchObjectInternal()) { + Locks.withLock(getLockInternal(), () -> { // hold synch locks in this order to prevent deadlock synchronized (children) { if (Entities.isAncestor(this, child)) throw new IllegalStateException("loop detected trying to add child "+child+" to "+this+"; it is already an ancestor"); @@ -659,7 +664,7 @@ public abstract class AbstractEntity extends AbstractBrooklynObject implements E sensors().emit(AbstractEntity.CHILD_ADDED, child); } } - } + }); return child; } @@ -688,7 +693,7 @@ public abstract class AbstractEntity extends AbstractBrooklynObject implements E @Override public boolean removeChild(Entity child) { - synchronized (getAttributesSynchObjectInternal()) { + return Locks.withLock(getLockInternal(), () -> { synchronized (children) { boolean changed = children.remove(child); child.clearParent(); @@ -702,7 +707,7 @@ public abstract class AbstractEntity extends AbstractBrooklynObject implements E } return changed; } - } + }); } // -------- GROUPS -------------- @@ -1103,14 +1108,18 @@ public abstract class AbstractEntity extends AbstractBrooklynObject implements E if (LOG.isTraceEnabled()) LOG.trace(""+AbstractEntity.this+" setAttributeWithoutPublishing "+attribute+" "+val); - T result = attributesInternal.updateWithoutPublishing(attribute, val); - if (result == null) { - // could be this is a new sensor - entityType.addSensorIfAbsentWithoutPublishing(attribute); - } - - getManagementSupport().getEntityChangeListener().onAttributeChanged(attribute); - return result; + // internal code done on rebind, locking a bit redundant, but strictly speaking + // all updates should have the lock, and the sensor add should be done in same lock block + return Locks.withLock(getLockInternal(), () -> { + T result = attributesInternal.updateInternalWithoutLockOrPublish(attribute, val); + if (result == null) { + // could be this is a new sensor + entityType.addSensorIfAbsentWithoutPublishing(attribute); + } + + getManagementSupport().getEntityChangeListener().onAttributeChanged(attribute); + return result; + }); } @Beta @@ -1131,24 +1140,29 @@ public abstract class AbstractEntity extends AbstractBrooklynObject implements E LOG.trace(message); } } - T result = attributesInternal.modify(attribute, modifier); - if (result == null) { - // could be this is a new sensor - entityType.addSensorIfAbsent(attribute); - } - - // TODO Conditionally set onAttributeChanged, only if was modified - getManagementSupport().getEntityChangeListener().onAttributeChanged(attribute); - return result; + // ensure sensor added in same lock block + return Locks.withLock(getLockInternal(), () -> { + T result = attributesInternal.modify(attribute, modifier); + if (result == null) { + // could be this is a new sensor + entityType.addSensorIfAbsent(attribute); + } + + // TODO Conditionally set onAttributeChanged, only if was modified + getManagementSupport().getEntityChangeListener().onAttributeChanged(attribute); + return result; + }); } @Override public void remove(AttributeSensor<?> attribute) { if (LOG.isTraceEnabled()) LOG.trace(""+AbstractEntity.this+" removeAttribute "+attribute); - - attributesInternal.remove(attribute); - entityType.removeSensor(attribute); + // removal should be done in same lock block + Locks.withLock(getLockInternal(), () -> { + attributesInternal.remove(attribute); + entityType.removeSensor(attribute); + }); } @Override http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/22178b65/core/src/main/java/org/apache/brooklyn/core/entity/EntityInternal.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/entity/EntityInternal.java b/core/src/main/java/org/apache/brooklyn/core/entity/EntityInternal.java index 46d10ab..87d92ee 100644 --- a/core/src/main/java/org/apache/brooklyn/core/entity/EntityInternal.java +++ b/core/src/main/java/org/apache/brooklyn/core/entity/EntityInternal.java @@ -178,7 +178,8 @@ public interface EntityInternal extends BrooklynObjectInternal, EntityLocal, Reb public interface SensorSupportInternal extends Entity.SensorSupport { /** * - * Like {@link EntityLocal#setAttribute(AttributeSensor, Object)}, except does not publish an attribute-change event. + * Like {@link #set(AttributeSensor, Object)}, except does not publish an attribute-change event. + * Used for rebinding. */ <T> T setWithoutPublishing(AttributeSensor<T> sensor, T val); http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/22178b65/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/LocalSubscriptionManager.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/LocalSubscriptionManager.java b/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/LocalSubscriptionManager.java index a726059..623fb26 100644 --- a/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/LocalSubscriptionManager.java +++ b/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/LocalSubscriptionManager.java @@ -44,6 +44,7 @@ import org.apache.brooklyn.api.sensor.SensorEventListener; import org.apache.brooklyn.core.entity.Entities; import org.apache.brooklyn.core.entity.EntityInternal; import org.apache.brooklyn.core.mgmt.BrooklynTaskTags; +import org.apache.brooklyn.core.sensor.AttributeMap; import org.apache.brooklyn.core.sensor.BasicSensorEvent; import org.apache.brooklyn.util.collections.MutableList; import org.apache.brooklyn.util.collections.MutableMap; @@ -63,6 +64,14 @@ import com.google.common.collect.Multimaps; /** * A {@link SubscriptionManager} that stores subscription details locally. + * + * Synchronization model: methods in this class synch on this object to ensure + * subscription order and delivery order on publish. + * <p> + * Frequently it will be called by a thread holding a lock on a value + * (eg {@link AttributeMap}, this synchronized methods here should not + * call to any value that may require that lock. + * In particular notifications of initial value should not look up values. */ public class LocalSubscriptionManager extends AbstractSubscriptionManager { http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/22178b65/core/src/main/java/org/apache/brooklyn/core/sensor/AttributeMap.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/sensor/AttributeMap.java b/core/src/main/java/org/apache/brooklyn/core/sensor/AttributeMap.java index d9da6ae..1986a40 100644 --- a/core/src/main/java/org/apache/brooklyn/core/sensor/AttributeMap.java +++ b/core/src/main/java/org/apache/brooklyn/core/sensor/AttributeMap.java @@ -23,11 +23,15 @@ import static com.google.common.base.Preconditions.checkNotNull; import java.util.Collection; import java.util.Collections; import java.util.Map; +import java.util.concurrent.Callable; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import org.apache.brooklyn.api.entity.Entity; import org.apache.brooklyn.api.sensor.AttributeSensor; import org.apache.brooklyn.core.BrooklynLogging; import org.apache.brooklyn.core.entity.AbstractEntity; +import org.apache.brooklyn.util.concurrent.Locks; import org.apache.brooklyn.util.core.flags.TypeCoercions; import org.apache.brooklyn.util.guava.Maybe; import org.slf4j.Logger; @@ -81,13 +85,32 @@ public final class AttributeMap { this.values = checkNotNull(storage, "storage map must not be null"); } - /** Internal object this class synchs on when modifying values. - * Exposed for internal usage to synchronize on this to enforce canonical order. - * @return + /** + * This externally visible lock ensures only one thread can write and publish + * any sensor value at a time. Methods which set, modify, and publish values + * acquire this lock. + * <p> + * Reads are not blocked by this, although this synchs on {@link #values} + * when actually changing the low-level value to ensure consistency there. + * <p> + * See {@link #getLockInternal()} + */ + private final transient ReentrantLock writeLock = new ReentrantLock(); + + /** Internal {@link Lock} used when publishing values. + * When needed -- and carefully -- callers can lock on this to ensure no values are changed by other + * threads while they are active. + * <p> + * To prevent deadlock, Brooklyn code generally call this _before_ getting any lower-level + * monitor (ie {@code synchronized}) and before locking on any descendant entity. + * <p> + * In other words, this lock should generally *not* be acquired inside a {@code synchronized} block + * nor by the holder of a similar {@link Lock} on an entity except self or ancestor, + * unless a clear canonical order is set forth. */ @Beta - public Object getSynchObjectInternal() { - return values; + public Lock getLockInternal() { + return writeLock; } public Map<Collection<String>, Object> asRawMap() { @@ -115,8 +138,12 @@ public final class AttributeMap { * @param newValue the new value * @return the old value. * @throws IllegalArgumentException if path is null or empty + * + * @deprecated since 0.13.0 becoming private; callers should only ever use {@link #update(AttributeSensor, Object)} */ + @Deprecated // TODO path must be ordered(and legal to contain duplicates like "a.b.a"; list would be better + // (is there even any point to the path? it was for returning maps by querying a prefix but that's ancient!) public <T> T update(Collection<String> path, T newValue) { checkPath(path); @@ -138,44 +165,65 @@ public final class AttributeMap { Preconditions.checkArgument(!path.isEmpty(), "path can't be empty"); } + /** + * Sets a value and published it. + * <p> + * Constraints of {@link #getLockInternal()} apply, with lock-holder being the supplied {@link Function}. + */ public <T> T update(AttributeSensor<T> attribute, T newValue) { // 2017-10 this was unsynched which meant if two threads updated // the last publication would not correspond to the last value. - // could introduce deadlock but emit internal and publish should - // not seek any locks. _subscribe_ and _delivery_ might, but they - // won't be in this block. an issue with _subscribe-and-get-initial_ - // should be resolved by initial subscription queueing the publication - // to a context where locks are not held. - synchronized (values) { - T oldValue = updateWithoutPublishing(attribute, newValue); + // a crude `synchronized (values)` could cause deadlock as + // this does publish (doing `synch(LSM)`) whereas _subscribe_ + // would have the LSM lock and try to `synch(values)` + // as described at https://issues.apache.org/jira/browse/BROOKLYN-544. + // the addition of getLockInternal should make this much cleaner, + // as no one holding `synch(LSM)` should be updating here, + // ands gets here won't call any other code at all + return withLock(() -> { + T oldValue = updateInternalWithoutLockOrPublish(attribute, newValue); entity.emitInternal(attribute, newValue); return oldValue; - } + }); } + /** @deprecated since 0.13.0 this is becoming an internal method, {@link #updateInternalWithoutLockOrPublish(AttributeSensor, Object)} */ + @Deprecated public <T> T updateWithoutPublishing(AttributeSensor<T> attribute, T newValue) { - if (log.isTraceEnabled()) { - Object oldValue = getValue(attribute); - if (!Objects.equal(oldValue, newValue != null)) { - log.trace("setting attribute {} to {} (was {}) on {}", new Object[] {attribute.getName(), newValue, oldValue, entity}); - } else { - log.trace("setting attribute {} to {} (unchanged) on {}", new Object[] {attribute.getName(), newValue, this}); + return updateInternalWithoutLockOrPublish(attribute, newValue); + } + + @Beta + public <T> T updateInternalWithoutLockOrPublish(AttributeSensor<T> attribute, T newValue) { + synchronized (values) { + if (log.isTraceEnabled()) { + Object oldValue = getValue(attribute); + if (!Objects.equal(oldValue, newValue != null)) { + log.trace("setting attribute {} to {} (was {}) on {}", new Object[] {attribute.getName(), newValue, oldValue, entity}); + } else { + log.trace("setting attribute {} to {} (unchanged) on {}", new Object[] {attribute.getName(), newValue, this}); + } } + + T oldValue = update(attribute.getNameParts(), newValue); + return (isNull(oldValue)) ? null : oldValue; } - - T oldValue = update(attribute.getNameParts(), newValue); - - return (isNull(oldValue)) ? null : oldValue; } + private <T> T withLock(Callable<T> body) { return Locks.withLock(getLockInternal(), body); } + private void withLock(Runnable body) { Locks.withLock(getLockInternal(), body); } + /** - * Where atomicity is desired, the methods in this class synchronize on the {@link #values} map. + * Where atomicity is desired, this method wraps an acquisition of {@link #getLockInternal()} + * while applying the given {@link Function}. + * <p> + * Constraints of {@link #getLockInternal()} apply, with lock-holder being the supplied {@link Function}. */ public <T> T modify(AttributeSensor<T> attribute, Function<? super T, Maybe<T>> modifier) { - synchronized (values) { + return withLock(() -> { T oldValue = getValue(attribute); Maybe<? extends T> newValue = modifier.apply(oldValue); - + if (newValue.isPresent()) { if (log.isTraceEnabled()) log.trace("modified attribute {} to {} (was {}) on {}", new Object[] {attribute.getName(), newValue, oldValue, entity}); return update(attribute, newValue.get()); @@ -183,17 +231,25 @@ public final class AttributeMap { if (log.isTraceEnabled()) log.trace("modified attribute {} unchanged; not emitting on {}", new Object[] {attribute.getName(), newValue, this}); return oldValue; } - } + }); } + /** Removes a sensor. The {@link #getLockInternal()} is acquired, + * and constraints on the caller described there apply to callers of this method. + * <p> + * The caller is responsible for any subsequent actions needed, including publishing + * the removal of the sensor and triggering persistence. Caller may wish to + * have the {@link #getLockInternal()} while doing that. */ public void remove(AttributeSensor<?> attribute) { BrooklynLogging.log(log, BrooklynLogging.levelDebugOrTraceIfReadOnly(entity), "removing attribute {} on {}", attribute.getName(), entity); - - remove(attribute.getNameParts()); + withLock(() -> remove(attribute.getNameParts()) ); } // TODO path must be ordered(and legal to contain duplicates like "a.b.a"; list would be better + /** @deprecated since 0.13.0 becoming private; callers should only ever use {@link #remove(AttributeSensor)} + */ + @Deprecated public void remove(Collection<String> path) { checkPath(path); @@ -212,9 +268,6 @@ public final class AttributeMap { * @throws IllegalArgumentException path is null or empty. */ public Object getValue(Collection<String> path) { - // TODO previously this would return a map of the sub-tree if the path matched a prefix of a group of sensors, - // or the leaf value if only one value. Arguably that is not required - what is/was the use-case? - // checkPath(path); Object result = values.get(path); return (isNull(result)) ? null : result; http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/22178b65/core/src/main/java/org/apache/brooklyn/entity/group/AbstractGroupImpl.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/entity/group/AbstractGroupImpl.java b/core/src/main/java/org/apache/brooklyn/entity/group/AbstractGroupImpl.java index 81f6da4..5f2b51d 100644 --- a/core/src/main/java/org/apache/brooklyn/entity/group/AbstractGroupImpl.java +++ b/core/src/main/java/org/apache/brooklyn/entity/group/AbstractGroupImpl.java @@ -32,6 +32,7 @@ import org.apache.brooklyn.core.entity.EntityInternal; import org.apache.brooklyn.core.entity.lifecycle.ServiceStateLogic; import org.apache.brooklyn.core.mgmt.internal.ManagementContextInternal; import org.apache.brooklyn.entity.stock.DelegateEntity; +import org.apache.brooklyn.util.concurrent.Locks; import org.apache.brooklyn.util.exceptions.Exceptions; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -94,7 +95,7 @@ public abstract class AbstractGroupImpl extends AbstractEntity implements Abstra */ @Override public boolean addMember(Entity member) { - synchronized (getAttributesSynchObjectInternal()) { + return Locks.withLock(getLockInternal(), () -> { synchronized (members) { if (Entities.isNoLongerManaged(member)) { // Don't add dead entities, as they could never be removed (because addMember could be called in @@ -134,7 +135,7 @@ public abstract class AbstractGroupImpl extends AbstractEntity implements Abstra } return changed; } - } + }); } // visible for rebind @@ -149,7 +150,7 @@ public abstract class AbstractGroupImpl extends AbstractEntity implements Abstra */ @Override public boolean removeMember(final Entity member) { - synchronized (getAttributesSynchObjectInternal()) { + return Locks.withLock(getLockInternal(), () -> { synchronized (members) { boolean changed = (member != null && members.remove(member)); if (changed) { @@ -203,7 +204,7 @@ public abstract class AbstractGroupImpl extends AbstractEntity implements Abstra return changed; } - } + }); } @Override @@ -213,7 +214,7 @@ public abstract class AbstractGroupImpl extends AbstractEntity implements Abstra @Override public void setMembers(Collection<Entity> mm, Predicate<Entity> filter) { - synchronized (getAttributesSynchObjectInternal()) { + Locks.withLock(getLockInternal(), () -> { synchronized (members) { log.debug("Group {} members set explicitly to {} (of which some possibly filtered)", this, members); List<Entity> mmo = new ArrayList<Entity>(getMembers()); @@ -231,7 +232,7 @@ public abstract class AbstractGroupImpl extends AbstractEntity implements Abstra getManagementSupport().getEntityChangeListener().onMembersChanged(); } - } + }); } @Override http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/22178b65/core/src/test/java/org/apache/brooklyn/core/entity/AttributeMapTest.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/apache/brooklyn/core/entity/AttributeMapTest.java b/core/src/test/java/org/apache/brooklyn/core/entity/AttributeMapTest.java index 77ba9c6..b41f50a 100644 --- a/core/src/test/java/org/apache/brooklyn/core/entity/AttributeMapTest.java +++ b/core/src/test/java/org/apache/brooklyn/core/entity/AttributeMapTest.java @@ -142,11 +142,14 @@ public class AttributeMapTest { assertEquals(map.getValue(ImmutableList.of("b","c")), "2val"); } + // TODO we can simply remove this test when the deprecated method is removed; + // its behaviour is tested by other methods here + @SuppressWarnings("deprecation") @Test public void testStoredByPathCanBeRetrieved() throws Exception { AttributeSensor<String> sensor1 = Sensors.newStringSensor("a", ""); AttributeSensor<String> sensor2 = Sensors.newStringSensor("b.c", ""); - + map.update(ImmutableList.of("a"), "1val"); map.update(ImmutableList.of("b", "c"), "2val"); http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/22178b65/utils/common/src/main/java/org/apache/brooklyn/util/concurrent/Locks.java ---------------------------------------------------------------------- diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/concurrent/Locks.java b/utils/common/src/main/java/org/apache/brooklyn/util/concurrent/Locks.java new file mode 100644 index 0000000..6efef08 --- /dev/null +++ b/utils/common/src/main/java/org/apache/brooklyn/util/concurrent/Locks.java @@ -0,0 +1,47 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.brooklyn.util.concurrent; + +import java.util.concurrent.Callable; +import java.util.concurrent.locks.Lock; + +import org.apache.brooklyn.util.exceptions.Exceptions; + +public class Locks { + + public static <T> T withLock(Lock lock, Callable<T> body) { + try { + lock.lockInterruptibly(); + } catch (InterruptedException e) { + throw Exceptions.propagate(e); + } + try { + return body.call(); + } catch (Exception e) { + throw Exceptions.propagate(e); + } finally { + lock.unlock(); + } + } + + public static void withLock(Lock lock, Runnable body) { + withLock(lock, () -> { body.run(); return null; } ); + } + +} http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/22178b65/utils/common/src/test/java/org/apache/brooklyn/util/collections/LocksTest.java ---------------------------------------------------------------------- diff --git a/utils/common/src/test/java/org/apache/brooklyn/util/collections/LocksTest.java b/utils/common/src/test/java/org/apache/brooklyn/util/collections/LocksTest.java new file mode 100644 index 0000000..251037f --- /dev/null +++ b/utils/common/src/test/java/org/apache/brooklyn/util/collections/LocksTest.java @@ -0,0 +1,37 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.brooklyn.util.collections; + +import java.util.concurrent.locks.ReentrantLock; + +import org.apache.brooklyn.util.concurrent.Locks; +import org.testng.Assert; +import org.testng.annotations.Test; + +@Test +public class LocksTest { + + public void withLockTest() { + ReentrantLock l = new ReentrantLock(); + Assert.assertTrue(Locks.withLock(l, () -> l.isHeldByCurrentThread())); + Locks.withLock(l, () -> Assert.assertTrue(l.isHeldByCurrentThread())); + Assert.assertFalse(l.isHeldByCurrentThread()); + } + +}
