See inline... On 07/31/2013 06:08 AM, Sanne Grinovero wrote:
> # Hibernate: > > 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 a > 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. DriverManagerConnectionProviderImpl. Stopping it should not generally be run concurrent with other accesses, but can't hurt to synchronize it anyway... > - Most of the Inconsistent Sync findings are likely false positives due > 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 the > field 'collectedParameterSpecifications' in the class > 'QueryTranslatorImpl'. 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 "Inconsistent Sync". > - The non-atomic check/put in 'StatefulPersistenceContext' looks like a > true positive. Not sure what this means. StatefulPersistenceContext is huge and has 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 for > the ones in 'AuditProcessManager', 'ColumnNameCache', and > 'DataSourceBasedMultiTennantConnectionProviderImpl', where the mapping > being stored in the concurrent hash maps looks like it is deterministic. Not 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 > inconsistent state. 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 name->EntityManagerFactory? Scott? _______________________________________________ hibernate-dev mailing list hibernate-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev
