On 07/31/2013 06:08 AM, Sanne Grinovero wrote:
Probably the best finding is the 'Shared non-thread-safe content'
finding in the class 'EntityManagerFactoryRegistry'. In general, the
inconsistent and mixed synchronisation findings are not very good, but
the (get/)check/put and thread-safe collection consistently guarded
findings look good.
- The ConcurrentModificationException finding looks like the catch was
intentional to test for the absence of an old bug.
I don't understand this
one. Could you explain any more?
- The only Inconsistent Collection Sync finding that looks like it is
true positive is 'pool' in the class
'DriverManagerConnectionImplProvider'. However, the unsynchronised
access is in a 'stop()' method and it is not clear whether this can run
concurrently with anything else.
it should not generally
be run concurrent with other accesses, but can't hurt to synchronize it
- Most of the Inconsistent Sync findings are likely false positives
to the unsynchronised accesses occurring in what the comments say is
test code. Better findings are on the field 'noTennantState', in the
classes 'LegacyHiLoAlgorithmOptimizer' and 'PooledLoOptimizer', and
field 'collectedParameterSpecifications' in the class
Not sure what this means. Looking at
PooledLoOptimizer, for example,
yes I see that noTenantState is generated under sync. Where is it
accessed outside a sync? Maybe I am just not understanding their phrase
- The non-atomic check/put in 'StatefulPersistenceContext'
looks like a
Not sure what this means. StatefulPersistenceContext is huge and
quite a few maps and quite a few access to those Maps. So (1) which
access(es) is this talking about and/or (2) what exactly is the problem
condition they are trying to describe here?
- The non-atomic get/check/puts all look like true positives, except
the ones in 'AuditProcessManager', 'ColumnNameCache', and
'DataSourceBasedMultiTennantConnectionProviderImpl', where the mapping
being stored in the concurrent hash maps looks like it is deterministic.
sure what this means
- The shared non-thread-safe content finding looks like it spots a
symptom of a real bug: in the method 'getNamedEntityManagerFactory', a
hashmap is extracted from a concurrent hash map and read from.
Concurrently, there is the possibility that items are removed from the
hash maps. There is no common lock guarding these accesses. This may
cause, at worst, infinite loops, if the hashmap is accessed in an
I actually have no idea why it keeps a Set for each name.
Seems to me
the code ultimately throws an exception anyway if more than one was
registered under that name, so why not just store