Hi,
On 20/04/16 15:54, "Alfusainey Jallow" wrote:
>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.
the synchronized block is not there to protected a shared resource
but allows us to use the AtomicBoolean as a monitor object for
thread communication (wait/notify).
>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.
this is intentional. using the DocumentNodeStore object itself for
synchronization can be troublesome as well. Client code could try
to synchronize on the node store as well and interfere with internal
code synchronizing on the same object. That's why I prefer to use
private monitor object like DocumentNodeStore#backgroundReadMonitor.
Regards
Marcel
