I might have missed some context, but a side note: > If current thread is write lock owner then it can't acquire read lock. > RRWL doesn't allow upgrades or downgrades.
>From ReentrantReadWriteLock javadoc: Reentrancy also allows downgrading from the write lock to a read lock, by acquiring the write lock, then the read lock and then releasing the write lock. However, upgrading from a read lock to the write lock is not possible. Best regards, Ivan Pavlukhin вт, 25 февр. 2020 г. в 22:05, Andrey Gura <ag...@apache.org>: > > Agree with Andrey and Slava. > > Your change could lead to exception in case when current thread does > not own read lock. > > > Checking for a writе lock holding through method isHeldByCurrentThread is > > not atomic, so there is no certainty that this thread still owns a write > > lock. > > What do you mean when talk "method isHeldByCurrentThread is not > atomic"? There is no need in any additional guarantees here because > current thread always sees own changes and any other thread could not > change thread reference if current thread is owner. Also current > thread can't check lock owning and release lock concurrently. > > > I assume that a read lock was obtained and in this code we did not release > > it. > > If current thread is write lock owner then it can't acquire read lock. > RRWL doesn't allow upgrades or downgrades. > > Stack traces from JIRA issue don't point to deadlock. "Deadlock: true" > means that JVM detected deadlock but it doesn't mean that exactly this > threads in deadlock. > > On Tue, Feb 25, 2020 at 9:25 PM Andrey Mashenkov > <andrey.mashen...@gmail.com> wrote: > > > > Write lock is an exclusive lock and no other threads can obtain read or > > write lock while current thread holding write lock. > > > > Why you think checking for current thread is lock-owner is not atomic? > > A thread that already holds a write lock has no needs to obtain read locks > > unless it releases the write-lock. > > > > Your change can have serious impact. > > Can you share a reproducer? > > > > > > вт, 25 февр. 2020 г., 20:42 Дмитрий Горчаков < > > dmitry.gorchakov.mu...@gmail.com>: > > > > > > Could you please describe in detail the issue you are trying to fix? > > > > > > I once got same deadlock on a productive in v2.7.5. New iteration of > > > checkpoint could not start. Dump attached. > > > > > > > By the way, please take a look at > > > GridCacheDatabaseSharedManager.checkpointReadLock: > > > > > > Thanks, I will look more closely at this check. > > > > > > > So, do we really need to unconditionally unlock the read lock at > > > checkpointReadUnlock as you propose? > > > > > > Checking for a writе lock holding through method isHeldByCurrentThread is > > > not atomic, so there is no certainty that this thread still owns a write > > > lock. > > > I assume that a read lock was obtained and in this code we did not release > > > it. > > > > > > Thanks! > > > > > > вт, 25 февр. 2020 г. в 14:35, Вячеслав Коптилин <slava.kopti...@gmail.com > > > >: > > > > > >> Hello Dmitry, > > >> > > >> To be honest, I don't quite understand the proposed fix. Could you please > > >> describe in detail the issue you are trying to fix? > > >> By the way, please take a look at > > >> GridCacheDatabaseSharedManager.checkpointReadLock: > > >> > > >> @Override public void checkpointReadLock() { > > >> if (checkpointLock.writeLock().isHeldByCurrentThread()) > > >> return; > > >> ... > > >> } > > >> > > >> In other words, the read lock is not acquired in case the write lock is > > >> already held. > > >> So, do we really need to unconditionally unlock the read lock at > > >> checkpointReadUnlock as you propose? > > >> > > >> Thanks, > > >> S. > > >> > > >> вт, 25 февр. 2020 г. в 13:32, Dmitry Gorchakov < > > >> dmitry.gorchakov.mu...@gmail.com>: > > >> > > >> > This issue once reproduced in my productive with similar stack trace. > > >> > During > > >> > sources analysis I saw the use unsafe of method > > >> > writeLock().isHeldByCurrentThread() for unlocking checkpoint. This may > > >> > cause > > >> > thread to starvation. Problem no longer appeared after local fix. > > >> > I prepared pull request https://github.com/apache/ignite/pull/7445. > > >> > Accept please jira issue to me and review my PR. > > >> > > > >> > > > >> > > > >> > > > >> > -- > > >> > Sent from: http://apache-ignite-developers.2346864.n4.nabble.com/ > > >> > > > >> > > >