RFR: 8369631: Assess and remedy any unsafe usage of the sr_semaphore Semaphore in the Posix signal code

David Holmes dholmes at openjdk.org
Tue Oct 14 20:23:04 UTC 2025


On Tue, 14 Oct 2025 09:44:52 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:

>> Replace the statically allocated "semaphore" with a pointer and lazily initialize it. I chose this approach as we already have a second Semaphore declared that way.
>> 
>> Testing:
>> - tiers 1-3 sanity
>> 
>> Thanks.
>
> src/hotspot/os/posix/signals_posix.cpp line 172:
> 
>> 170:   static OSXSemaphore* sr_semaphore;
>> 171: #else
>> 172:   static PosixSemaphore* sr_semaphore;
> 
> I don't think consistency with the other semaphore is a great reason for
> avoiding the use of `DeferredStatic` here. Indeed, I would prefer that
> `DeferredStatic` be used for both semaphores.
> 
> I think the only change from what's already being proposed here to make
> `sr_semaphore` be `DeferredStatic` is the declaration and the initialization.
> 
> The changes needed for `sig_semaphore` to be `DeferredStatic` are pretty
> small. Change the declaration and initialization, and os::signal_notify needs
> to be adjusted. The last I think actually makes the code clearer and more
> consistent.
> 
> 
>  void os::signal_notify(int sig) {
> -  if (sig_semaphore != nullptr) {
> +  if (!ReduceSignalUsage) {
>      AtomicAccess::inc(&pending_signals[sig]);
>      sig_semaphore->signal();
> -  } else {
> -    // Signal thread is not created with ReduceSignalUsage and jdk_misc_signal_init
> -    // initialization isn't called.
> -    assert(ReduceSignalUsage, "signal semaphore should be created");
>    }
>  }
> 
> 
> That would change a too-early signal to a crash in a product build, except our
> signal handlers are installed after the semaphores are initialized.

Thanks for taking a look @kimbarrett . I expected someone would say "why not switch both to DeferredStatic?" and I don't have an issue doing that (missing init checks notwithstanding), but it wasn't my immediate goto because I don't see `DeferredStatic as really being any superior to declaring a pointer instead (which is how we do lazy initialization in 99.9% of the VM).

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

PR Review Comment: https://git.openjdk.org/jdk/pull/27762#discussion_r2430345486


More information about the hotspot-runtime-dev mailing list