Need help with ZGC failure in Lilliput

[Roman Kennke]

Alright, I disabled monitor deflation, and the problem persists.

I narrowed the problem down to two (closely related) calls to 
ZUtils::object_size() (which in turn calls oopDesc::size() which calls 
oopDesc::klass()), which seem to trigger it: ZLiveMap::iterate() and 
ZRelocateClosure::relocate_object(). When I change those two calls to 
use the Klass* from the dedicated field, the failure disappears. I don't 
think that the actual Klass* disagrees (I have asserts to check that), 
which means it can only be the side effect of that call -- most likely 
the header inflation. How this inflation would affect ZGC is still not 
entirely clear to me.

The problem always happens during weak storage scan. Inflation of a lock 
creates new weak handle that points back to the object. I suspect that, 
at the very least, this must not point to the old copy of that object. I 
need to think this through.

Roman

> Hi Erik,
> 
> yes I have thought about this, but I am not sure if what I do is enough. 
> I'm basically following the logic that is implemented for hash-code: if 
> we encounter a stack-lock in the current thread, we can directly load 
> the displaced-header from it, otherwise it inflates the lock. I guess 
> that this is not really going perfect with GCs, because it means that, 
> e.g., concurrent marking or relocation would inflate some locks.
> 
> The trouble might indeed be that GC threads would not be allowed to do 
> that because of concurrent deflation. Hrmpf. Is there a way to prevent 
> deflation during certain GC phases maybe? Or coordinate GC threads with 
> deflation?
> 
> Robbin Ehn's work sounds promising. Can you give me more details about 
> what he's up to? Maybe a PR?
> 
> Thanks,
> Roman
> 
>> Hi Roman,
>>
>> Might need to catch up a bit with what you have done in detail. 
>> However, rather than studying your changes in detail, I'm just gonna 
>> throw out a few things that I know would blow up when doing something 
>> like this, unless you have done something to fix that.
>>
>> The main theme is displaced mark words. Due to issues with displaced 
>> mark words, I don't currently know of any (good and performant) way of 
>> having any stable bits in the markWord that can be read by mutators, 
>> when there are displaced mark words. This is why in the generational 
>> version of ZGC we are currently building, we do not encode any age 
>> bits in the markWord. Instead it is encoded as a per-region/page 
>> property, which is more reliable, and can use much lighter 
>> synchronization. Anyway, here comes a few displaced mark word issues:
>>
>> 1) A displaced mark word can point into an ObjectMonitor. However, the 
>> monitor can be concurrently deflated, and subsequently freed. The safe 
>> memory reclamation policy for ObjectMonitor unlinks the monitors 
>> first, then performs a thread-local handshake with all (...?!) 
>> threads, and then frees them after the handshake, when it knows that 
>> surely nobody is looking at these monitors any longer. Except of 
>> course concurrent GC threads do not take part in such handshakes, and 
>> therefore, concurrent GC threads are suddenly unable to safely read 
>> klasses, through displaced mark words, pointing into concurrently 
>> freeing ObjectMonitors. It can result in use-after-free. They are 
>> basically not allowed to dereference ObjectMonitor, without more 
>> synchronization code to allow that.
>>
>> 2) A displaced mark word can also point into a stack lock, right into 
>> the stack of a concurrently running thread. Naturally, this thread can 
>> concurrently die, and its stack be deallocated, or concurrently 
>> mutated after the lock is released on that thread. In other words, the 
>> memory of the stack on other threads is completely unreliable. The way 
>> in which this works regarding hashCode, which similarly needs to be 
>> read by various parties, is that the stack lock is concurrently 
>> inflated into an inflated lock, which is then a bit more stable to 
>> read through, given the right sync dance. Assuming of course, that the 
>> reading thread, takes part in the global handshake for SMR purposes.
>>
>> So yeah, not sure if you have thought about any of this. If not, it 
>> might be the issue you are chasing after. It's worth mentioning that 
>> Robbin Ehn is currently removing displaced mark words with his Java 
>> monitors work. That should make this kind of exercise easier.
>>
>> Thanks,
>> /Erik
>>
>> On 2021-07-20 13:47, Roman Kennke wrote:
>>> Hi ZGC devs,
>>>
>>> I am struggling with a ZGC problem in Lilliput, and would like to ask 
>>> for your opinion.
>>>
>>> I'm currently working on changing runtime oopDesc::klass() to load 
>>> the Klass* from the object header instead of the dedicated Klass* field:
>>>
>>> https://github.com/openjdk/lilliput/pull/12
>>>
>>> This required some coordination in other GCs, because it's not always 
>>> safe to access the object header. In particular, objects may be 
>>> locked, at which point we need to find the displaced header, or worst 
>>> case, inflate the header. I believe I've solved that in all GCs.
>>>
>>> However, I am still getting a failure with ZGC, which is kinda 
>>> unexpected, because it's the only GC that is *not* messing with 
>>> object headers (as far as I know. If you check out the above PR, the 
>>> failure can easily reproduced with:
>>>
>>> make run-test TEST=gc/z/TestGarbageCollectorMXBean.java
>>>
>>> (and only that test is failing for me).
>>>
>>> The crash is in ZHeap::is_object_live() because the ZPage there turns 
>>> out to be NULL. I've added a bunch of debug output in that location, 
>>> and it looks like the offending object is always inflated *and* 
>>> forwarded when it happens, but I fail to see how this is related to 
>>> each other, and to the page being NULL. I strongly suspect that 
>>> inflation of the object header by calling klass() on it causes the 
>>> troubles. Changing back to original implementation of 
>>> oopDesc::klass() (swap commented-out-code there) makes the bug 
>>> disappear.
>>>
>>> Also, the bug always seems to happen when calling through a weak 
>>> barrier. Not sure if that is relevant.
>>>
>>> Any ideas? Opinions?
>>>
>>> Thanks,
>>> Roman
>>>
>>