RFR: 8373100: Genshen: Control thread can miss allocation failure notification [v2]

[Y. Srinivas Ramakrishna]

On Fri, 5 Dec 2025 18:53:37 GMT, William Kemper <wkemper at openjdk.org> wrote:

>> In some cases, the control thread may fail to observe an allocation failure. This results in the thread which failed to allocate waiting forever for the control thread to run a cycle. Depending on which thread fails to allocate, the process may not make progress.
>
> William Kemper has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Set requested gc cause under a lock when allocation fails

Thanks for cleaning this up.

Did you review the non-generational ShenandoahControlThread and uses thereof to make sure a similar issues doesn't exist there?

As Xiaolong states, it might be worthwhile to do a refactor that shares as much as needed and no more, and to do so cleanly.

This looks good; sorry for the delay in reviewing.

🚢

src/hotspot/share/gc/shenandoah/shenandoahGenerationalControlThread.hpp line 143:

> 141:   void notify_control_thread(MonitorLocker& ml, GCCause::Cause cause, ShenandoahGeneration* generation);
> 142:   void notify_control_thread(GCCause::Cause cause);
> 143:   void notify_control_thread(MonitorLocker& ml, GCCause::Cause cause);

Nit:

I'd (subjectively) order them thus: (nct = notify_control_thread)

1. nct(cause)
2. nct(ml, cause)
3. nct(cause, generation)
4. nct(ml, cause, generation)

For completeness in the documentation comment preceding, state that if an argument, cause or generation, is missing, it isn't updated.

I am assuming that there is a specific small subset of cause values where the generation isn't important to spell out and really implies "isn't necessary or is implicitly understood" for cancellation/request cause? Is there a call argument/consistency check that might be done in the nct:s where these bottom out to confirm this, or am I being unnecessarily paranoid?

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

Marked as reviewed by ysr (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/28665#pullrequestreview-3573943496
PR Review Comment: https://git.openjdk.org/jdk/pull/28665#discussion_r2615874522