RFR: 8351484: Race condition in max stats in MonitorList::add

Patricio Chilano Mateo pchilanomate at openjdk.org
Mon Mar 10 17:32:54 UTC 2025


On Mon, 10 Mar 2025 11:30:50 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

> See bug for the description of the race. New gtest test case reliably catches fire on my 5950X desktop without this fix.
> 
> I needed to add the proper constructor for `MonitorList` to make it testable. In our regular code, it is statically allocated, so its fields are implicitly initialized.
> 
> I also made counter updates `_relaxed` for completeness.
> 
> Additional testing:
>  - [x] Linux x86_64 server fastdebug, new test now passes
>  - [x] GHA

Looks good to me.

test/hotspot/gtest/runtime/test_synchronizer.cpp line 81:

> 79: 
> 80:   // Something to reference in OM. It makes no difference which oop it is,
> 81:   // as long as it is correct.

Can’t we still safepoint on `workers.doit()` when calling `VMThreadBlocker::start()`? Maybe we could use a Handle to be safe. Sidenote: This is pre-existent but isn’t `TestThreadGroup::doit()` missing a call to `_blocker->ready()` before starting the worker threads?

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

PR Review: https://git.openjdk.org/jdk/pull/23961#pullrequestreview-2671728147
PR Review Comment: https://git.openjdk.org/jdk/pull/23961#discussion_r1987731880


More information about the hotspot-runtime-dev mailing list