[
https://issues.apache.org/jira/browse/SLIDER-1242?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16155454#comment-16155454
]
Billie Rinaldi commented on SLIDER-1242:
----------------------------------------
I missed one use of containsKey in ComponentTagProvider.
> Review uses of double-checked locking
> -------------------------------------
>
> Key: SLIDER-1242
> URL: https://issues.apache.org/jira/browse/SLIDER-1242
> Project: Slider
> Issue Type: Bug
> Reporter: Billie Rinaldi
> Assignee: Billie Rinaldi
> Fix For: Slider 1.0.0
>
> Attachments: SLIDER-1242.1.patch
>
>
> There are several places where we perform double-checked locking. Even though
> the practice is discouraged, I believe it is technically correct when the
> check is performed on the presence of a ConcurrentHashMap key, which is how
> we are using it.
> However, in two places, AgentProviderService#getCurrentExports and
> AgentProviderService#getAllocatedPorts, containsKey is used instead of get to
> perform the check. I am seeing some indication that containsKey is not
> sufficient, and that get must be used for double-checked locking to be
> correct. There is a comment in the ConcurrentHashMap#containsKey method that
> says "same as get() except no need for volatile value read" -- and I think
> that volatile value read is what we need for correctness.
> Also, in the [ConcurrentHashMap api
> doc|https://docs.oracle.com/javase/8/docs/api/java/util/concurrent/ConcurrentHashMap.html],
> it specifically mentions get and does not mention containsKey: "Any non-null
> result returned from get(key) and related access methods bears a
> happens-before relation with the associated insertion or update" and "an
> update operation for a given key bears a happens-before relation with any
> (non-null) retrieval for that key reporting the updated value."
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
