RFR: 8235324: Dying objects are published from users of CollectedHeap::object_iterate

[Per Liden]

Hi Stefan,

On 12/4/19 11:34 AM, Stefan Karlsson wrote:
> Removed the unused oopDesc::keep_alive function:
> 
> https://cr.openjdk.java.net/~stefank/8235324/webrev.02.delta/
> https://cr.openjdk.java.net/~stefank/8235324/webrev.02/

zHeap.cpp should include zBarrier.inline.hpp now.

I would maybe like to restructure the ZGC keepalive barrier a bit (or at 
least discuss some ideas). For example, I'm thinking the fast path could 
be "is_null_or_not_during_mark". But let's take that as a separate 
discussion/change.

So, other than the missing include, looks good. No need for a new webrev.

cheers,
/Per

> 
> StefanK
> 
> On 2019-12-04 09:50, Stefan Karlsson wrote:
>> Hi all,
>>
>> Please review this fix to make sure objects published from closures to 
>> CollectedHeap::object_iterate are kept alive.
>>
>> https://cr.openjdk.java.net/~stefank/8235324/webrev.01/
>> https://bugs.openjdk.java.net/browse/JDK-8235324
>>
>> Caution needs to be taken when CollectedHeap::object_iterate is used. 
>> It may:
>> - Return non-reachable objects - for example, when G1 walks the prev 
>> bitmap
>> - Return the Reference.referent without an appropriate resurrection 
>> barrier
>> - Return an object loaded without a "strong"/"marking" barrier
>>
>> These objects should not escape the confines of the active safepoint 
>> that calls object_iterate. If they do we run the risk of various 
>> crashes when the objects get garbage collected.
>>
>> Unfortunately, there are a few closures that do this. The once we have 
>> identified are:
>> - HeapInspection::find_instances_at_safepoint
>> - JVMTI object tagging
>>
>> There might be some merit to solving this by altering the 
>> object_iterate API, but for now we'd like to fix the two problematic 
>> areas listed above. The suggestion is to introduce a 
>> CollectedHeap::keep_alive function that notifies the GC that the 
>> object should be kept alive the current GC cycle (if active).
>>
>> This solution will also work with code that iterates over object 
>> fields without using object_iterate. For example, with this fix we'll 
>> be able to read the Reference.referent field in JDK-8234508, with 
>> AS_NO_KEEPALIVE, and only keep the object alive if it actually gets 
>> registered in the tag map. Without that, every single object reachable 
>> through Reference.referent would be kept alive.
>>
>> The fix introduces a keep_alive() function for ZGC, G1, and 
>> Shenandoah. It would be good to get reviews from maintainers of all 
>> these GCs.
>>
>> Thanks,
>> StefanK
>