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