On Fri, 10 Jul 2026 06:52:03 GMT, Serguei Spitsyn <[email protected]> wrote:

>> This update fixes the test bug which was initially filed as a Valhalla bug 
>> but then found to be NOT Valhalla specific.
>> The static variable `objectCount` is used for two different purposes:
>>  - to count current walked objects number
>>  - as a number to tag objects
>> 
>> This leads to mismatches in indexes calculations.
>> For instance the `ObjectFree` event callback decrements the `objectCount` 
>> number. Then the `objectCountMax` can be smaller than some of the actual 
>> indexes.
>> The fix does:
>>  - adds a fragment to force GC's with the JVMTI `ForceGarbageCollection` to 
>> make the failure well reproducible
>>  - split `objectCount` into: `objectCount` and `objectTagCount`
>>  - `objectTagCount` is used to set the object tags and value of the 
>> `objectCountMax`
>>  - a couple of the `ind` boundary checks are also corrected
>> 
>> Testing:
>>  - Executed the test locally: 
>> `jvmti/IterateOverReachableObjects/iterreachobj002`
>>  - TBD: Submit mach5 tiers 1-3 to be safe
>> 
>> ---------
>> - [x] I confirm that I make this contribution in accordance with the 
>> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai).
>
> Serguei Spitsyn has updated the pull request with a new target base due to a 
> merge or a rebase. The incremental webrev excludes the unrelated changes 
> brought in by the merge/rebase. The pull request contains three additional 
> commits since the last revision:
> 
>  - Merge
>  - review: moved to the right place the line which sets the objectCountMax
>  - 8384882: jvmti/IterateOverReachableObjects/iterreachobj002 fails with Not 
> a valid malloc pointer

Sorry Serguei I thought I could help out here but I don't understand the code 
enough. I don't quite understand how the counts are used. The tagcount seems to 
increase forever and is used to size the array by setting the max-count to its 
current value - but when does this happen and how-often? The objCount is the 
next index into the array IIUC but it can be reduced when an object is freed. 
Seems to me the array could potentially be much larger than the maximum index 
actually used.

test/hotspot/jtreg/vmTestbase/nsk/jvmti/IterateOverReachableObjects/iterreachobj002/iterreachobj002.cpp
 line 42:

> 40: static volatile long objectCount = 0;
> 41: static volatile long objectCountMax = 0;
> 42: static volatile long objectTagCount = 0;

Pre-existing but we shouldn't be using `long`  due to different size on 
Windows. I find the long/jlong conversions a bit hard to follow as well.

test/hotspot/jtreg/vmTestbase/nsk/jvmti/IterateOverReachableObjects/iterreachobj002/iterreachobj002.cpp
 line 79:

> 77:     /* Set tag */
> 78:     *tag_ptr = (jlong)++objectTagCount;
> 79:     objectCount++;

Shouldn't we only increment this if the allocation succeeds?

test/hotspot/jtreg/vmTestbase/nsk/jvmti/IterateOverReachableObjects/iterreachobj002/iterreachobj002.cpp
 line 115:

> 113: */
> 114:     if (ind < 0 || ind >= objectCountMax) {
> 115:         NSK_COMPLAIN1("heapRootCallbackForSecondObjectsIteration: 
> invalid object tag value: %d\n", (long)*tag_ptr);

Format specifier should be `%ld` for a long

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

PR Review: https://git.openjdk.org/jdk/pull/31585#pullrequestreview-4681102785
PR Review Comment: https://git.openjdk.org/jdk/pull/31585#discussion_r3567460582
PR Review Comment: https://git.openjdk.org/jdk/pull/31585#discussion_r3567478831
PR Review Comment: https://git.openjdk.org/jdk/pull/31585#discussion_r3567481931

Reply via email to