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 [email protected] or file a JIRA ticket
with INFRA.
---
