review (S) for 6975006: assert(check.is_deoptimized_frame()) failed: missed deopt

[Tom Rodriguez]

I discovered a problem with this code that necessitated a small change.  Basically the code shouldn't be used when frame::deoptimize isn't called from a safepoint, which occurs as part of the fix for 6902182 where we stopped forcing a safepoint for all deoptimization.  Basically the whole deopt suspend logic can be skipped in that case because you can't be in thread_in_native when deoptimizing yourself.  Tested with full nsk with -XX:+DeoptimizeALot -XX:CompileThreshold=100.  http://cr.openjdk.java.net/~never/6975006.

tom

On Aug 9, 2010, at 4:52 AM, Christian Thalinger wrote:

> On Thu, 2010-08-05 at 13:25 -0700, Tom Rodriguez wrote:
>> http://cr.openjdk.java.net/~never/6975006
>> 
>> 6975006: assert(check.is_deoptimized_frame()) failed: missed deopt
>> Reviewed-by:
>> 
>> The safepointing logic treats threads that are thread_in_native as if
>> they are halted since the Java state is safe while we are in native
>> state.  If the thread happens to return from native during the
>> safepoint it will simply come to a halt.  On sparc this creates some
>> complexity when patching for deoptimization because the return address
>> is kept in a register and only flushed to stack by the chip.  We force
>> flushing of the windows in the JNI stub but because of the way
>> register windows work this doesn't help the frame that is just above a
>> native wrapper since the window might be on chip while the native
>> wrapper itself is executing.  There's machinery in the deopt code that
>> detects the case where the caller of a native wrapper is the one being
>> deoptimized and arranges for the native wrapper to rewrite the return
>> address when it comes out of native.  The problem is that this code
>> examines the current state of the thread at the time the deopt occurs
>> not what state was when the safepoint started.  This creates a little
>> race where a native wrapper might come to a halt on it's own after the
>> safepoint started but before the deopt patching occurred, which
>> sidesteps the deopt suspend logic because it's not in one of the
>> thread_in_native states.  The fix is to record the state of the thread
>> at the beginning of the safepoint and consult that when triggering the
>> deopt suspend logic.
>> 
>> Tested by repeatedly running test with -XX:+DeoptimizeALot.
>> Previously it would fail within 5 minutes but after the fix it ran
>> overnight until I simply killed it.
> 
> Looks good.  -- Christian
>