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