brooklyn-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From aledsage <...@git.apache.org>
Subject [GitHub] incubator-brooklyn pull request: Bunch of fixes observed during ho...
Date Sat, 18 Oct 2014 11:59:39 GMT
Github user aledsage commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/255#discussion_r19053258
  
    --- Diff: core/src/main/java/brooklyn/location/basic/SshMachineLocation.java ---
    @@ -219,6 +222,22 @@ public SshMachineLocation(Map properties) {
             usedPorts = (usedPorts != null) ? Sets.newLinkedHashSet(usedPorts) : Sets.<Integer>newLinkedHashSet();
         }
     
    +    @Override
    +    public void init() {
    +        super.init();
    +    }
    +
    +    private final transient Object poolCacheMutex = new Object();
    +    private LoadingCache<Map<String, ?>, Pool<SshTool>> getSshPoolCache()
{
    +        synchronized (poolCacheMutex) {
    --- End diff --
    
    Regarding @neykov 's comment "Personally prefer double null check (out of the sync block
as well) so that we need synchronization on first call only but fine as is."
    
    If you do ever use double-check locking like that, then be sure to declare your field
volatile (which then removes much of the performance benefit of the double-check locking).
The only time you can get away without the volatile is if it's a primitive (that is 32 bits
or less). Otherwise, you risk other threads seeing a semi-initialised object because the writer
thread and the second thread don't have a "happens before" relationship.
    
    Double-checked locking is broken (according to the Java memory model spec) in far too
many uses of it, but folk "get away with it" (i.e. hardly ever see problems) because of the
way the current generations of Intel multi-core chips behave. It's usage is often not worth
the performance gain (and consider alternatives like instantiating the field in the constructor,
declared final).
    
    But such comments aren't relevant for this particular usage obviously!


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

Mime
View raw message