Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/96#discussion_r58174385
--- Diff:
core/src/main/java/org/apache/brooklyn/core/entity/AbstractEntity.java ---
@@ -1548,7 +1550,7 @@ public boolean unsubscribe(Entity producer,
SubscriptionHandle handle) {
* @deprecated since 0.9.0; for internal use only
*/
@Deprecated
- protected synchronized SubscriptionTracker getSubscriptionTracker() {
+ protected SubscriptionTracker getSubscriptionTracker() {
--- End diff --
the `BasicSubscriptionSupport.getSubscriptionTracker()` method this calls
also synchronizes on `AbstractEntity.this` (line 1494 in new money) -- so no
impact whether this method is `synchronized` or not
in contrast to the change to unsynchronize `getSubscriptionContext()` here,
synchronizing on *something* when getting the tracker looks right as it is
setting a local field. however the underlying method (line 1498 new money)
calls to `getSubscriptionContext()` while synchronized on `this` so i think we
haven't entirely eliminated the deadlock prospect.
feels like line 1498 could double-check the tracker, and load the
`getSubscriptionContext()` between the checks eg
if (tracker!=null) return tracker;
subs = getSubscriptionContext();
synchronized (AbstractEntity.this) {
if (tracker!=null) return tracker;
tracker = new Tracker(subs);
return tracker;
}
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
