On Wed, 18 Dec 2024 09:12:55 GMT, Joakim Nordström <jnordst...@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 I've noted that the `open/test/hotspot/jtreg/runtime/signal` test is flawed, and doesn't really test that the signal chaining actually does what it should (see [JDK-8257644](https://bugs.openjdk.org/browse/JDK-8257644)). None the less, the added warning verification checks does work. ------------- PR Comment: https://git.openjdk.org/jdk/pull/22806#issuecomment-2550803999