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

Reply via email to