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

[Xin Liu]

On Sat, 20 Aug 2022 22:43:03 GMT, Man Cao <manc 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

src/hotspot/os/posix/signals_posix.cpp line 1348:

> 1346:     // We also use os::signal() and a dummy handler to avoid special-casing
> 1347:     // set_signal_handler() and JVM_HANDLE_XXX_SIGNAL().
> 1348:     os::signal(BREAK_SIGNAL, CAST_FROM_FN_PTR(void*, DummySIGBREAKHandler));

Bummer. This is what I tried to avoid. Sorry!

By moving sigaction of BREAK_SIGNAL out of the "window" -- [begin_signal_setting, end_signal_setting], `libjsig` will not record the old action of 'BREAK_SIGNAL', so HotSpot still has freedom to replace it with **UserHandler**, which can handle ThreadDump request.

I agree that your approach is less intrusive and is better than previous one!

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.

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'.

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

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