RFR: 8292695: SIGQUIT and jcmd attaching mechanism does not work with signal chaining library

[Man Cao]

On Sun, 21 Aug 2022 06:46:11 GMT, Xin Liu <xliu at openjdk.org> wrote:

>> Hi all,
>> 
>> Could anyone review this bug fix? See https://bugs.openjdk.org/browse/JDK-8292695 for details.
>> 
>> I changed the temporary handler for SIGQUIT to use a dummy function, and use `os::signal()` to set it up, just as `os::initialize_jdk_signal_support()` does.
>> It is possible that just moving the `set_signal_handler(BREAK_SIGNAL, false);` in `install_signal_handlers()` outside of the window bounded by `JVM_{begin|end}_signal_setting()` could also fix this bug. However, `set_signal_handler()` and `JVM_HANDLE_XXX_SIGNAL()` are currently used for signals that support chaining and periodically check, which do not apply to SIGQUIT. I think it is cleaner to use different functions for SIGQUIT.
>> 
>> I also added a test to check that sending SIGQUIT should produce a thread dump on stdout, with and without using libjsig.so.
>> 
>> -Man
>
> test/hotspot/jtreg/runtime/Thread/TestBreakSignalThreadDump.java line 51:
> 
>> 49: import jdk.test.lib.process.ProcessTools;
>> 50: import jdk.test.lib.process.OutputAnalyzer;
>> 51: import vm.share.ProcessUtils;
> 
> Can we reference vmTestbase in jtreg test? I search runtime and other directories,  this is the first case to do so. 
> 
> How about we implement 'ProcessTools.sendCtrlBreak(Process)' in test/lib? We can leave it unimplemented on Windows.
> 
> Not only it can test your case, but it could also test JDK-8279124. Options like `-Xms8g -XX:+AlwaysPreTouch -XX:ParallelGCThreads=1 -Xlog:gc+heap=debug` at least takes 5s. Parent process can send SIGQUIT and check whether JVM intercepts it.

There are a few serviceability dcmd jtreg tests that already depend on /vmTestbase:
- test/hotspot/jtreg/serviceability/dcmd/framework/HelpTest.java
- test/hotspot/jtreg/serviceability/dcmd/framework/VMVersionTest.java
- test/hotspot/jtreg/serviceability/dcmd/framework/Invalidcommandtest.java

Is there a real downside to depend on /vmTestbase?

Agree that it is a good suggestion to add a parameter to ProcessTools.sendCtrlBreak() and move it to test/lib. This function is only used in one other place datadumpreq001.java, so it's probably easy to change.
However, it's probably better to make a separate test-only RFE to move this function and add a test for JDK-8279124 together.

> test/hotspot/jtreg/runtime/Thread/TestBreakSignalThreadDump.java line 56:
> 
>> 54: 
>> 55:     static class TestProcess {
>> 56: 	static {
> 
> It looks like you embed 'tab' rather than 'whitespaces'.

Sorry, I used wrong coding style for formatter. Fixed.

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

PR: https://git.openjdk.org/jdk/pull/9955