On Mon, 6 Jan 2025 04:27:23 GMT, David Holmes <dhol...@openjdk.org> wrote:
>> Could I get a review of this fix to refine the warnings printed by `libjsig` >> when using the deprecated `signal()`/`sigset()` functions? >> >> Currently the libjsig library supports chaining `signal()` and `sigset()`. >> With these functions being deprecated, `libjsig` warns when a program calls >> either function when the it's LD_PRELOADed. The warning is supposed to warn >> that when the support for `signal()` and `sigset()` is removed, the >> application might stop working as expected. >> >>> VM warning: the use of signal() and sigset() for signal chaining was >>> deprecated in version 16.0 and will be removed in a future release. Use >>> sigaction() instead. >> >> The warning is however printed in a few cases when its unnecessary, and also >> fails to catch some usages where it should be printed. With this fix the >> warning is printed (in order as they appear in jsig.c): >> 1. when `signal()` or `sigset()` is called for a JVM-used signal, after the >> JVM has installed its signals >> 2. when the JVM installs its signals (using `sigaction()`), and libjsig sees >> that `signal()` or `sigset()` has been used for a JVM-used signal >> >> ### Changes >> * The printing is removed from `call_os_signal()`. This warning was only >> correct if `signal()` or `sigset()` was called for a JVM-used signal before >> the JVM was installed. This is now instead covered by step 1 above. >> * If the JVM was installed first, the `signal()`/`sigset()` calls were >> effectively translated to `sigaction()`, and the warning was _not_ printed. >> When the `signal()`/`sigset()` support is dropped, this behaviour will >> change and the warning should be printed. This is now covered by step 2. >> >> ## Testing >> - [x] Test `open/test/hotspot/jtreg/runtime/signal` has added checks for the >> deprecation warning, and passes with the fix >> - [x] Tier1-3 in progress > > test/hotspot/jtreg/runtime/signal/SigTestDriver.java line 176: > >> 174: /** >> 175: * Return true for all signals used by the JVM. This only covers the >> 176: * case where the JVM is started normally, and not with -Xrs. > > The VM also uses SIGUSR2 (`SR_signum`), SIGINT, and SIGQUIT. Though the > latter two only if no `-Xrs` is given. The test is skipped for SIGUSR2 on Linux and MacOS, and Windows skips all signal testing. So I guess that leaves aix, which seems to use signals_poxis.cpp... so perhaps the SIGUSR2 test should be skipped for aix too? But I'll add the signals for clarity. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/22806#discussion_r1905534105
