review (S) for 6656830: assert((*p)->is_oop(), "expected an oop while scanning weak refs")

[Christian Thalinger]

On Fri, 2010-06-18 at 11:55 -0700, Tom Rodriguez wrote:
> While investigating the sweeper bug I ended up diagnosing this one too.  I already have one review from Dan since he helped me work it out.
> 
> http://cr.openjdk.java.net/~never/6656830
> 
> 6656830: assert((*p)->is_oop(),"expected an oop while scanning weak refs")
> Reviewed-by: dcubed
> 
> This is a long standing bug with the handling of jmethodIDs by the
> logic for notifying of code unloading.  The compiled method unload
> event passes the jmethodID as part of the event so you need to look it
> up when an nmethod is unloaded because the class it's associated with
> is being unloaded.  jmethodIDs are loosely coupled to the methodOop so
> there's an array of them in the instanceKlass and they are searched
> when the jmethodID for a methodOop is required.  jmethodIDs are really
> just weak JNI references so when their class is being unloaded they
> all become null so it's impossible to look up the right jmethodID for
> an nmethod which is being unloaded.  In the failing test this results
> in a new jmethodID being allocated in the middle of the GC which can
> later lead to crashes since root set has been extended so stale oops
> are being processed in later phases of the GC.  The simplest fix is to
> cache the jmethodID in the nmethod so that it can be used in the
> unload notification.
> 
> I also added logic to complain if new weak references are created
> while a GC is active.

There is a small typo:

src/share/vm/code/nmethod.cpp:

1528   // that the jmethodID is a weak referemce to the methodOop so if

-- Christian