Usage of Collections.unmodifiableCollection is unsafe 60/51960/1
authorTom Pantelis <tpanteli@brocade.com>
Wed, 15 Feb 2017 05:06:39 +0000 (00:06 -0500)
committerTom Pantelis <tpanteli@brocade.com>
Thu, 16 Feb 2017 13:20:20 +0000 (13:20 +0000)
commitc71ecaf8a0d2d04c343dbfec0a9cfd5162c277f6
tree3784998f784a54a153f34a12b84c11efa7abd490
parent8dfdfb5627c0434a4d253945a8f590f9c66f4777
Usage of Collections.unmodifiableCollection is unsafe

This is a follow-up to https://git.opendaylight.org/gerrit/#/c/51583/
wrt a review comment questioning why I returned a new ArrayList instead
of returning the Set directly. One reason was to avoid mutation of the
internal set by the caller but also to capture the state of the set at
that point in time and avoid concurrent mods which may not be safe.
Where a concurrent set is used, it would be thread-safe to return the
set directly but the set may by modified as the caller is iterating it
which may not be desired. For the other class whose set is a keySet
from a non-threadsafe Map, the caller could get a ConcurrentMod
exception while iterating. Based on the review, I changed the call
sites to return a Collections.unmodifiableCollection but this is also
incorrect and is susceptible to the same issues as that impl reads-thru
to the underlying collection. Therefore I changed the call sites back
to returning a new ArrayList.

Change-Id: I504f38c5bfc4c707180ac301eb10acd0ac24f872
Signed-off-by: Tom Pantelis <tpanteli@brocade.com>
(cherry picked from commit d98bea5d456be31205b64dc8f0c5f3ae2b1a4cd0)
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/DataChangeListenerSupport.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/DataTreeChangeListenerSupport.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/DataTreeCohortActorRegistry.java