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

[Vladimir Kozlov]

Looks good.

Vladimir

Tom Rodriguez wrote:
> 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
>>
>