RFR: 8373039: Remove Incorrect Asserts in shenandoahScanRemembered

Y. Srinivas Ramakrishna ysr at openjdk.org
Wed Dec 3 18:30:29 UTC 2025


On Wed, 3 Dec 2025 17:45:31 GMT, William Kemper <wkemper at openjdk.org> wrote:

>> The `Klass->is_valid` asserts in this file do not hold the required `ClassLoaderDataGraph_lock` and can cause a crash.
>> 
>> A similar issue was seen in https://bugs.openjdk.org/browse/JDK-8372566
>> 
>> This change passes all tests in `TEST=hotspot_gc_shenandoah` with a fastdebug build
>
> src/hotspot/share/gc/shenandoah/shenandoahScanRemembered.cpp line 384:
> 
>> 382:   oop obj = cast_to_oop(p);
>> 383:   assert(oopDesc::is_oop(obj), "Should be an object");
>> 384:   assert(p <= left, "p should start at or before left end of card");
> 
> I think it's fine to take out this loop, but the assert on 384 now seems redundant to the assert on 363. I'm also not sure if the assert on 385 necessarily holds because `p` is no longer increased in the loop. Maybe remove this whole `#ifdef ASSERT` block, or leave in the loop and just take out the `Klass::is_valid` usage.

I agree. In addition, the comment should be updated so it doesn't make the confusing reference to "the loop that follows", which just went away, etc. It's fine to leave a suitably modified comment as to why it is safe to query the size of the object at the oop being returned.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/28642#discussion_r2586182968


More information about the shenandoah-dev mailing list