RFR: 8349094: GenShen: Race between control and regulator threads may violate assertions [v12]

[William Kemper]

On Fri, 14 Feb 2025 22:56:20 GMT, Y. Srinivas Ramakrishna <ysr at openjdk.org> wrote:

>> William Kemper has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 30 commits:
>> 
>>  - Merge remote-tracking branch 'jdk/master' into fix-control-regulator-threads
>>  - Old gen bootstrap cycle must make it to init mark
>>  - Merge remote-tracking branch 'jdk/master' into fix-control-regulator-threads
>>  - Improve message for assertion
>>  - Make shutdown safer for threads requesting (or expecting) gc
>>  - Do not accept requests if control thread is terminating
>>  - Notify waiters when control thread terminates
>>  - Add event for control thread state changes
>>  - Fix shutdown livelock error
>>  - Fix includes
>>  - ... and 20 more: https://git.openjdk.org/jdk/compare/ba6c9659...915ffbda
>
> src/hotspot/share/gc/shenandoah/shenandoahGenerationalControlThread.hpp line 64:
> 
>> 62: private:
>> 63:   // This lock is used to coordinate setting the _requested_gc_cause, _requested generation
>> 64:   // and _gc_mode. It is important that these be changed together and have a consistent view.
> 
> In that case, for ease of maintenance, I'd move the declaration of all of the 3 data members that this lock protects next to this lock, either immediately preceding or immediately succeeding its declaration in the body of this class.
> 
> Are these data members always both read and written under this lock? If so, then `_gc_mode` below doesn't need to be defined `volatile`.

The `_gc_mode` is read without the lock by the regulator thread. However, the regulator thread does take the lock and reads `_gc_mode` again under the lock before making any state changes.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23475#discussion_r1968132168