Hi,
I am currently going through the oak code base trying to address issues
reported by the findbugs analysis tool[1], some of which i try to address
in OAK-4257[2]. I did not want touch some of the concurrency issues
reported without first writing to the list.
In DocumentNodeStore (inside of oak-core plugins), syncronization is
performed on the 'isDisposed' instance field which is declared as
AtomicBoolean. now in the DocumentNodeStore#dispose() call, you have this:
// notify background threads waiting on isDisposed
synchronized (isDisposed) {
isDisposed.notifyAll();
}
Findbugs is warning about this saying:
This method performs synchronization an object that is an instance of a
> class from the java.util.concurrent package (or its subclasses). Instances
> of these classes have their own concurrency control mechanisms that are
> orthogonal to the synchronization provided by the Java keyword
> synchronized. For example, synchronizing on an AtomicBoolean will not
> prevent other threads from modifying the AtomicBoolean.
Such code may be correct, but should be carefully reviewed and documented,
> and may confuse people who have to maintain the code at a later date.
since i am not yet familiar with the code base, i decided to raise this
before attempting to fix it (synchronizing on *this* object instead?) or
may be this is intentional? the same issue is
in DocumentNodeStore.NodeStoreTask#run() call.
Most NPEs issues (possible null pointer derefence) are also reported in
many parts of the code base and most complains results in a call to
NodeBuilder#getProperty(String). this method can return null and the return
object should always be checked for null. however, most of its usage in the
code base does not explicitly check for a null-return value. is this
intentional too?
[1] https://analysis.apache.org/dashboard/index/113395
[2] https://issues.apache.org/jira/browse/OAK-4257
kind regards
--
alfu
