8216541: CompiledICHolders of VM locked unloaded nmethods are released too late

[Erik Österlund]

Hi Tobias,

Thanks for having a look at this.

On 2019-01-29 09:16, Tobias Hartmann wrote:
> Hi Erik,
> 
> very nice analysis, thanks a lot for investigating!
> 
> On 28.01.19 14:56, Erik Österlund wrote:
>> http://cr.openjdk.java.net/~eosterlund/8216541/webrev.00/
> 
> Why did you remove the call to thread->set_scanned_compiled_method(NULL) in sweeper.cpp?

Because the CompiledMethodMarker destructor already nulls this out, and 
redundantly nulling it out again offers no extra protection.

The idea of nulling it out before calling flush seems to have been to 
prevent the GC scanning from seeing this flushed nmethod in a safepoint, 
accidentally resurrecting it from the dead. But that is already 
impossible, because flush() is called with a never safepoint checking 
lock (which guarantees we don't have any and can't add any safepoint 
checks while holding that lock or we will deadlock badly). Therefore 
such safepoints will happen strictly after the processing of the 
compiled method is finished, and it is already cleared the normal way.

By removing that pointless clearing, I could get rid of the 
release_compiled_method() function and just call flush directly instead. 
I get confused by there being two "destroy" functions, one in the 
sweeper and one in the nmethod, so I wanted it gone.

> 
>> The proposed change has survived 200 rounds of kitchensink, hs-tier1-3 and hs-precheckin-comp.
> 
> In the meanwhile, could you please run some more 100x iterations of kitchensink?

Sure, running some more as we speak.

Thanks,
/Erik

> 
> Thanks,
> Tobias
>