Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/589#discussion_r28210046
--- Diff: core/src/main/java/brooklyn/util/config/ConfigBag.java ---
@@ -132,12 +166,12 @@ public String getDescription() {
/** current values for all entries
* @return non-modifiable map of strings to object */
- public Map<String,Object> getAllConfig() {
- return Collections.unmodifiableMap(config);
+ public synchronized Map<String,Object> getAllConfig() {
--- End diff --
+1 to phasing out the mutable accesses
as for `this`, it's a common and well understood pattern, and the sync
dangers are no different to elsewhere. there may be times someone wants to
batch items so it is handy to expose the object for synching. in time perhaps
we'll replace with a RW lock, which would be the elegant thing, but since most
`ConcurrentMap` impls don't do that it seems like overkill for now.
have added a comment in javadoc about synching and possibility of that
changing
---
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.
---
