[hibernate-dev] Static analysis report on thread safety of Hibernate
steve at hibernate.org
Wed Jul 31 09:40:26 EDT 2013
I can look after I get done mucking around with the JPA TCK...
On 07/31/2013 06:08 AM, Sanne Grinovero wrote:
> Hi all,
> the ThreadSafe team (http://contemplateltd.com) kindly offered a trial
> license to inspect Hibernate and Infinispan projects for code
> correctness; you can think of this as the "Findbugs for concurrent
> code". The idea is to mutually benefit from it: we get to use the
> tool, but we should also provide feedback to the dev team.
> Even better than just providing me with a key, they where kind enough
> to run it themselves on the Hibernate ORM and Hibernate Search
> codebases, and what follows is their own opinion; note that while they
> are expert in using the tool & in concurrency problems they need our
> help to verify if these are real bugs and eventually prioritize
> For example I see a comment about a non-safe usage of a
> PersistenceContext; AFAIK that's not supposed to be thread-safe so
> should not be a problem..
> I'll lead the analysis of the Hibernate Search report myself; could
> someone else please take the lead on the ORM report over; still if
> something isn't clear I'm glad to be involved, and please send me some
> feedback that I can then share to the development team of the tool.
> Sad note: please don't ask me to share the license, I'm not allowed,
> but I'll be happy to run it on other projects as well and share
> reports; we have until the 30th of August.
> A verbatim copy of the comments from the ThreadSafe team follow below.
> # 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.
> - The putIfAbsent findings all look like false positives, generally
> because the object construction part looks too heavyweight to benefit
> from putIfAbsent.
> - 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.
> - 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
> - All of the Mixed Synchronisation findings are unfortunately false
> positives, due to the guard infererence heuristics failing.
> - The non-atomic check/put in 'StatefulPersistenceContext' looks like a
> true positive.
> - 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.
> - 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.
> - All the Thread-safe collection consistently guarded findings look
> # Hibernate Search
> The best findings here are the asynchronous callback ones, while the
> inconsistent sync ones are all likely false positives, due to lifecycle
> - The putIfAbsent findings are generally best ignored here.
> - The inconsistent synchronisation findings are all likely false
> positives, due to lifecycle constraints.
> - The non atomic check/put findings both look good.
> - The Unsynchronised write to field from asynchronous callback findings
> are all symptoms of the same underlying bug: the execution of the
> stop() method may overlap with the lazily performed initialisation.
> hibernate-dev mailing list
> hibernate-dev at lists.jboss.org
More information about the hibernate-dev