Fwd: Patch: Clean up fix for JDK-8047720

[David Holmes]

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.holmes at oracle.com
> <mailto:david.holmes at 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
>
>