RFR: 8254042: gtest/GTestWrapper.java failed os.test_random

[Coleen Phillimore]

On 11/25/20 12:25 AM, Thomas Stuefe wrote:
> On Tue, 24 Nov 2020 23:38:52 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:
>
>> The function os::init_random() and os::random() both set the _rand_seed. This test thinks nothing can change the seed while it is computing its expected value.
>> I changed the test to run in a VM operation safepoint.  Alternately, I can change the test to not verify the random value computed in the loop, or remove the test.
>> Ran tier1 tests on linux-x64 and windows-x64.
> Hi Coleen,
>
> Why is someone concurrently changing the seed? I thought "TEST" tests do not start the VM? Or is it that some earlier test already did?

Yes, people investigated this and the VM is started by an earlier test.
>
> Could we not just change the test to use os::next_random(seed)?
> That would be a two-line change: remove the init_random call, and replace os::random() with `num = seed = os::next_random(seed);`

This is a really good suggestion.
>
> That said, moving the initialization out of the platform files and initialize the seed directly makes sense.

I'd like to keep this part because it seemed senseless to me, but I'll 
rework the rest.

>
> If you want to stay with your approach of making os::init_random() only available at safepoint, could you make it private? CDS could - or arguably should - use the same technique as outlined above, using os::next_random with an own seed instead of os::random.

I'd like only the os.random test to know about next_random function and 
leave CDS alone.

Thanks,
Coleen
>
> A third possibility would be to keep the seed THREAD_LOCAL.
>
> Cheers, Thomas
>
> -------------
>
> PR: https://git.openjdk.java.net/jdk/pull/1422