Dear David, Any news regarding this fix? Anything I can do to help?
Best wishes, Carsten On Wed, Feb 4, 2015 at 12:54 PM, Carsten Varming <varm...@gmail.com> wrote: > Dear David, > > I took a look at all occurrences of PeriodicTask_lock. We are in luck: > > PeriodicTask::enroll and PeriodicTask::disenroll check for safepoints when > acquiring PeriodicTask_lock. > > PeriodicTask::time_to_wait and PeriodicTask::real_time_tick are called > only by the watcher thread. > > WatcherThread::unpark is used only in contexts where PeriodicTask_lock is > owned by the caller. > > WatcherThread::sleep is called only by the watcher thread. > > I took another look at WatcherThread::sleep and realized that there is a > path from acquiring PeriodicTask_lock to waiting on the lock where > _should_terminate is not checked. I added that to the minimal fix attached. > > Looking at these methods made me want to clean up a little more. See > better.patch attached. > > I feel pretty good about the patch with the limited usage of > no_safepoint_check_flag with PeriodicTask_lock. > > Carsten > > > On Tue, Feb 3, 2015 at 11:28 PM, David Holmes <david.hol...@oracle.com> > wrote: > >> On 4/02/2015 1:28 PM, Carsten Varming wrote: >> >>> Dear David Holmes, >>> >>> Please see inline response, >>> >>> On Tue, Feb 3, 2015 at 9:38 PM, David Holmes <david.hol...@oracle.com >>> <mailto:david.hol...@oracle.com>> wrote: >>> >>> On 4/02/2015 5:00 AM, Carsten Varming wrote: >>> >>> Greetings all, >>> >>> I was recently introduced to the deadlock described in >>> JDK-8047720 and >>> fixed in JDK9. The fix seems a little messy to me, and it looks >>> like it >>> left some very short races in the code. So I decided to clean up >>> the >>> code. See attached diff. >>> >>> The change takes a step back and acquires PeriodicTask_lock in >>> WatcherThread::stop (like before the fix in JDK9), but this time >>> safepoints are allowed to proceed when acquiring >>> PeriodicTask_lock, >>> preventing the deadlock. >>> >>> >>> It isn't obvious that blocking for a safepoint whilst in this method >>> will always be a safe thing to do. That would need to be examined in >>> detail - potential interactions can be very subtle. >>> >>> >>> Absolutely true. For reference, the pattern is repeated with the >>> Terminator_lock a few lines below. The pattern is also used in >>> Threads::destroy_vm before and after calling before_exit, and the java >>> shutdown hooks are called right before the call to before_exit. So there >>> is a lot of evidence that safepoints are allowed to happen in this >>> context. >>> >> >> The thread calling before_exit is a JavaThread so of course it >> participates in safepoints. The issue is whether the interaction between >> that thread and the WatcherThread, via the PeriodicTask_lock, can allow for >> the JavaThread blocking at a safepoint whilst owning that lock. If another >> JavaThread can try to lock it without a safepoint check then we can get a >> deadlock. >> >> Cheers, >> David >> >> >> Thank you for taking your time to look at this, >>> Carsten >>> >>> >>> David >>> >>> >>> Let me know what you think, >>> Carsten >>> >>> >>> >
