[9] RFR(S): 8029443: 'assert(klass->is_loader_alive(_is_alive)) failed: must be alive' during VM_CollectForMetadataAllocation

[Tobias Hartmann]

Hi Mikael,

thanks for the review. Please see comments inline.

On 15.07.2014 13:36, Mikael Gerdin wrote:
> Tobias,
>
> On Tuesday 15 July 2014 09.48.08 Tobias Hartmann wrote:
>> Hi Vladimir,
>>
>>> Impressive work, Tobias!
>> Thanks! Took me a while to figure out what's happening.
>>
>>> So before the permgen removal embedded method* were oops and they were
>>> processed in relocInfo::oop_type loop.
>> Okay, good to know. That explains why the terms oops and metadata are
>> used interchangeably at some points in the code.
> Yep, there are a lot of leftover references to metadata as oops, especially in
> some compiler/runtime parts such as MDOs and CompiledICs.
>
>>> May be instead of specializing opt_virtual_call_type and
>>> static_call_type call site you can simple add a loop for
>>> relocInfo::metadata_type (similar to oop_type loop)?
>> The problem with iterating over relocInfo::metadata_type is that we
>> don't know to which stub, i.e., to which IC the Method* pointer belongs.
>> Since we don't want to unload the entire method but only clear the
>> corresponding IC, we need this information.
> I'm wondering, is there some way to figure out the IC for the Method*?
>
> In CompiledStaticCall::emit_to_interp_stub a static_stub_Relocation is created
> and from the looks of it it points to the call site through some setting of a
> "mark".
>
> The metadata relocation is emitted just after the static_stub_Relocation, so
> one approach (untested) could be to have a case for static_stub_Relocations,
> create a CompiledIC.at(reloc->static_call()) and check if it's a call to
> interpreted. If it is the advance the relocIterator to the next position and
> check that metadata for liveness.

The relocation entries for this particular case are [1]. Looking at the 
static_stub_Relocation (0xffffffff6ea49cc4) we don't know if the stub 
belongs to an optimized IC or a compiled static call. We would either 
have to create both CompiledIC.at(..) and compiledStaticCall_at(..) or 
check the relocation entry for the call (0xffffffff6ea49bc0) requiring 
another iteration. Only then we are able to look at the 
metadata_relocation at the next position.

Since we already have case statements for opt_virtual_call_type and 
static_call_type (at least in nmethod::do_unloading_parallel(..)) I 
would prefer to infer the Method* from the IC or compiled static call.

I adapted the implementation according to Coleen's feedback:

http://cr.openjdk.java.net/~thartmann/8029443/webrev.01/

What do you think?

Thanks,
Tobias


[1] Relocation entries:

[...]
154031          @0xffffffff6ea49b3a: 3005
154032 relocInfo at 0xffffffff6ea49b3a [type=3(opt_virtual_call) 
addr=0xffffffff6ea49bc0 offset=20]
[...]
154047          @0xffffffff6ea49b52: f801ffe8500a
154048 relocInfo at 0xffffffff6ea49b56 [type=5(static_stub) 
addr=0xffffffff6ea49cc4 offset=40 data=-24] | 
[static_call=0xffffffff6ea49bc0]
154049          @0xffffffff6ea49b58: f003c000
154050 relocInfo at 0xffffffff6ea49b5a [type=12(metadata) 
addr=0xffffffff6ea49cc4 offset=0 data=3] | 
[metadata_addr=0xffffffff6ea49d68 *=0xffffffff6ae20960 
offset=0]metadata_value=0xffffffff6ae20960: {method} 
{0xffffffff6ae20968} 'newInstance' 
'([Ljava/lang/Object;)Ljava/lang/Object;' in 
'sun/reflect/GeneratedConstructorAccessor3'
154051          @0xffffffff6ea49b5c: f003c007
154052 relocInfo at 0xffffffff6ea49b5e [type=12(metadata) 
addr=0xffffffff6ea49ce0 offset=28 data=3] | 
[metadata_addr=0xffffffff6ea49d68 *=0xffffffff6ae20960 
offset=0]metadata_value=0xffffffff6ae20960: {method} 
{0xffffffff6ae20968} 'newInstance' 
'([Ljava/lang/Object;)Ljava/lang/Object;' in 
'sun/reflect/GeneratedConstructorAccessor3'
154053          @0xffffffff6ea49b60:



> /Mikael
>
>> Thanks,
>> Tobias
>>
>>> Thanks,
>>> Vladimir
>>>
>>> On 7/14/14 4:56 AM, Tobias Hartmann wrote:
>>>> Hi,
>>>>
>>>> please review the following patch for JDK-8029443.
>>>>
>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8029443
>>>> Webrev: http://cr.openjdk.java.net/~thartmann/8029443/webrev.00/
>>>>
>>>> *Problem*
>>>> After the tracing/marking phase of GC, nmethod::do_unloading(..) checks
>>>> if a nmethod can be unloaded because it contains dead oops. If class
>>>> unloading occurred we additionally clear all ICs where the cached
>>>> metadata refers to an unloaded klass or method. If the nmethod is not
>>>> unloaded, nmethod::verify_metadata_loaders(..) finally checks if all
>>>> metadata is alive. The assert in CheckClass::check_class fails because
>>>> the nmethod contains Method* metadata corresponding to a dead Klass.
>>>> The Method* belongs to a to-interpreter stub [1] of an optimized
>>>> compiled IC. Normally we clear those stubs prior to verification to
>>>> avoid dangling references to Method* [2], but only if the stub is not in
>>>> use, i.e. if the IC is not in to-interpreted mode. In this case the
>>>> to-interpreter stub may be executed and hand a stale Method* to the
>>>> interpreter.
>>>>
>>>> *Solution
>>>> *The implementation of nmethod::do_unloading(..) is changed to clean
>>>> compiled ICs and compiled static calls if they call into a
>>>> to-interpreter stub that references dead Method* metadata.
>>>>
>>>> The patch was affected by the G1 class unloading changes (JDK-8048248)
>>>> because the method nmethod::do_unloading_parallel(..) was added. I
>>>> adapted the implementation as well.
>>>> *
>>>> Testing
>>>> *Failing test (runThese)
>>>> JPRT
>>>>
>>>> Thanks,
>>>> Tobias
>>>>
>>>> [1] see CompiledStaticCall::emit_to_interp_stub(..)
>>>> [2] see nmethod::verify_metadata_loaders(..),
>>>> static_stub_reloc()->clear_inline_cache() clears the stub