nifi-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From taftster <...@git.apache.org>
Subject [GitHub] nifi pull request: [NIFI-784] RouteOnAttribute Performance Improve...
Date Wed, 12 Aug 2015 13:59:59 GMT
Github user taftster commented on the pull request:

    https://github.com/apache/nifi/pull/71#issuecomment-130314880
  
    Why is propertyMap marked volatile?  The value is only ever set once at construction time.
    
    If the answer is because of thread safety, the contents of the HashMap are not "protected"
just because the reference to the map is marked volatile.  puts/gets to the map do not inherit
the memory barrier protections associated to the volatile reference.  c.f.  http://stackoverflow.com/questions/10357823/volatile-hashmap-vs-concurrenthashmap
    
    Maybe a review of the concurrency issues of this processor is in order before accepting
this merge request?  I'm pretty sure that, even though the class will mostly behave correctly
since values are set during OnScheduled and OnStopped, these are not "safely published" to
the map.  While unlikely, other threads could potentially see stale values in this map.
    
    Either this class should likely be using ConcurrentHashMap here, or it should republish
an entirely fresh map by calling "new HashMap()" instead of "clear()".


---
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