Carsten.

I've started diving into this issue. Please see the updates
that I've been making to:

JDK-8072439 fix for 8047720 may need more work
https://bugs.openjdk.java.net/browse/JDK-8072439

Dan


On 2/10/15 3:18 PM, Carsten Varming wrote:
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 <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.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





Reply via email to