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/
> > >> >
> > >>
> > >

Reply via email to