Hi Carsten,

On 11/02/2015 8:18 AM, Carsten Varming wrote:
Dear David,

Any news regarding this fix? Anything I can do to help?

The bug has been assigned to Dan Daugherty so it depends on his workload. Verifying the safety of the change is a time consuming task.

Thanks,
David

Best wishes,
Carsten

On Wed, Feb 4, 2015 at 12:54 PM, Carsten Varming <varm...@gmail.com
<mailto: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 <mailto: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>
            <mailto:david.holmes@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




Reply via email to