RFR[13]: 8227277: HeapInspection::find_instances_at_safepoint walks dead objects

[David Holmes]

Hi Erik,

On 5/07/2019 8:19 pm, Erik Österlund wrote:
> Hi,
> 
> In the HeapInspection::find_instances_at_safepoint function, the unsafe 
> heap iteration API (which also walks dead objects) is used to find 
> objects that are instance of a class, used for concurrent lock dumping 
> where we find dead 
> java.util.concurrent.locks.AbstractOwnableSynchronizer objects and 
> pointer chase to its possibly dead owner threadObj. There is a comment 
> saying that if this starts crashing because we use CMS, we should 
> probably change to use the safe_object_iterate() API instead, which does 
> not include dead objects.
> 
> Arguably, whether CMS is observed to crash or not, we really should not 
> be walking over dead objects and exposing them anyway. It's not safe... 
> and it will crash sooner or later.
> 
> For example, CMS yields to safepoints (including young GCs) while 
> sweeping. This means that both the AbstractOwnableSynchronizer and its 
> owner thread might have died, but while sweeping, we could yield for a 
> young GC that promotes objects overriding the memory of the dead thread 
> object with random primitives, but not yet freeing the dead 
> AbstractOwnableSynchronizer. A subsequent dumping operation could use 
> the heap walker to find the dead AbstractOwnableSynchronizer, and 
> pointer chase into its dead owner thread, which by now has been freed 
> and had its memory clobbered with primitive data.
> 
> This will all eventually end up in a glorious crash. So we shouldn't do 
> this.
> 
> Bug:
> https://bugs.openjdk.java.net/browse/JDK-8227277
> 
> Webrev:
> http://cr.openjdk.java.net/~eosterlund/8227277/webrev.00/

That seems eminently reasonable. :)

Are there any valid uses for the (unsafe) object_iterate?

Cheers,
David

> Thanks,
> /Erik