Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/860#discussion_r144225899
--- Diff:
core/src/main/java/org/apache/brooklyn/core/sensor/AttributeMap.java ---
@@ -138,62 +165,91 @@ private void checkPath(Collection<String> path) {
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)} */
--- End diff --
Should be `since 1.0.0`
---
