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

Kim Barrett kbarrett at openjdk.org
Tue Oct 14 09:48:39 UTC 2025


On Mon, 13 Oct 2025 06:52:32 GMT, David Holmes <dholmes 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;

pre-existing: Why is this conditionalized, rather than just using `Semaphore` (which is already being used
for `sig_semaphore`)?

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.

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

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


More information about the hotspot-runtime-dev mailing list