RFR(XS) 8029441: assert(!((nmethod*)_cb)->is_deopt_pc(_pc)) failed: invariant broken

[Daniel D. Daugherty]

On 7/28/16 12:53 PM, dean.long at oracle.com wrote:
> https://bugs.openjdk.java.net/browse/JDK-8029441
>
> http://cr.openjdk.java.net/~dlong/8029441/webrev/
>
> This assert happens if we are unlucky enough to be at the start of the 
> deopt handler when another thread is calling 
> JavaThread::pd_get_top_frame_for_profiling() on the first thread. In 
> this case, the frame isn't going to be patched, so the assert is not 
> helping.  I removed the assert, but added a new assert to patch_pc() 
> to make sure we don't try to patch an unpatchable frame.

Dusting off my stack sampling cobwebs...

src/cpu/sparc/vm/frame_sparc.cpp
     So frame::init() has an assert about the current compiled PC
     not being a deopt PC. The sampling thread runs into this assert()
     in JavaThread::pd_get_top_frame() when it's trying to construct
     the top frame.

     The target thread happens to be just starting a deopt so that
     first frame's PC is a deopt PC. Ouch! Very nice catch of race
     that's probably been there a very long time.

     Update: Not very long in it's current form:

     11069: assert(!((CompiledMethod*)_cb)->is_deopt_pc(_pc), "invariant 
broken");

     $ hg log -r 11069
     changeset:   11069:69d081845165
     user:        rbackman
     date:        Tue Apr 26 10:28:51 2016 +0200
     summary:     8151956: Support non-continuous CodeBlobs in HotSpot

     But very old in it's previous form:

     0:     assert(!((nmethod*)_cb)->is_deopt_pc(_pc), "invariant broken");

     So back to SCCS history:

     1.168.1.1 assert(!((nmethod*)_cb)->is_deopt_pc(_pc), "invariant 
broken");

     $ sp -r1.168.1.1 src/cpu/sparc/vm/frame_sparc.cpp
     src/cpu/sparc/vm/SCCS/s.frame_sparc.cpp:

     D 1.168.1.1 06/12/07 10:06:37 sgoldman 298 297 00087/00035/00497
     MRs:
     COMMENTS:
     6463133 - patchless deopt. Automatically convert frames so that if they
     are deoptimized users can only see the original pc. Cache the codeblob
     that matches a frame's pc.

     So the earlier form of the assert dates back to Steve G's work on 
deopt.


     As for the new assert() in frame::patch_pc(), it looks fine to me
     but the proof is in the (long term) testing!

     Again, nice catch!

Dan


>
> Tested with "hs-nightly-runtime", "hs-tier0-comp", and all the 
> individual tests where this crash was happening.
>
> dl
>