Investigate JVM signal handling functions

[mgaudet]

The implementations of all 3 Signal JVM functions:

• JVM_RegisterSignal
• JVM_FindSignal
• JVM_RaiseSignal

Are questionable. All three need to be validated and completed.

[DanHeidinga]

The implementations of all 3 Signal JVM functions:

• JVM_RegisterSignal
• JVM_FindSignal
• JVM_RaiseSignal

Are questionable. All three need to be validated and completed. My vague understanding is that J9 relies on patches to the JCL code to work around the need for these 3 functions.

[mgaudet]

I'll re-title this as "Investigate JVM signal handling", as this came from me looking at https://github.com/eclipse/openj9/issues/40

[mstoodle]

I marked as "enhancement" but if "bug" becomes more appropriate we should update it :)

[pshipton]

@babsingh is this resolved by #283?

[babsingh]

@pshipton no. more changes will be required to complete this. currently, the following scenario is not covered.

public class SigIntTest extends Thread {

        private static Object monitor = new Object();

        public static void main(String args[]) {
                Runtime.getRuntime().addShutdownHook(new SigIntTest());
                System.out.println("Press CTRL-C to exit - Sends SIGINT signal");

                synchronized (monitor) {
                        try {
                                monitor.wait();
                        } catch (InterruptedException e) {
                                System.out.println("Main thread interrupted");
                        }
                        System.out.println("Terminating main thread correctly");
                }
        }

        public void run() {
                System.out.println("OK ... exiting");
                synchronized (monitor) {
                        monitor.notify();
                }
        }

}

Current behavior:

Press CTRL-C to exit
^C

Expected behavior:

Press CTRL-C to exit
^COK... exiting
Terminating main thread correctly

[DanHeidinga]

Current approach is https://github.com/eclipse/omr/issues/2332

[pshipton]

The initial changes are delivered to enable signal handling on Windows, Linux, and AIX. There will be more internal changes but the shutdown hook functionality should be working as of now.

Linux/AIX eclipse/omr#2494 #1603 #1642 Windows #1816 not the full functionality but enough for CTRL-C to trigger the shutdown hooks

[babsingh]

Adding @ibm-rtvs

[pshipton]

The nightly builds from May 4th https://adoptopenjdk.net/nightly.html?variant=openjdk8-openj9 should have the shutdown hook updates.

[SueChaplain]

@pshipton is there more testing to do here or can we remove this issue from the release notes in 0.9.0 please?

[pshipton]

There are more changes to be delivered before signal handling is fully functional, such as #1832

[pshipton]

1832 is merged.

[pshipton]

fyi #1984, it seems signals are not working as well as hoped.

edit: this is merged.

[pshipton]

Another fix here https://github.com/eclipse/omr/pull/2611

[thjaeckle]

Oh noo, the fix won't be in 0.9.0?

[pshipton]

Oh noo, the fix won't be in 0.9.0?

The eclipse/omr#2611 fix (and everything previous) is delivered and included in 0.9.0. I temporarily used this Issue in the 0.9.0 milestone because I couldn't add the omr issue directly, and was too lazy to create another issue in OpenJ9 to track it. Now that its been merged, any remaining work on signals is deferred until the next release. See the details in the release notes #1996

[pshipton]

Oops, release notes are here #1996

[pshipton]

I don't think this is going to be ready for the 0.10.0 release, deferring.

[babsingh]

@pshipton what's the deadline for OpenJ9 0.10.0 release?

signal handling works on win32 and win64amd. these platforms will be fully supported once https://github.com/eclipse/omr/pull/2908 (signal chaining changes) are merged.

Unix variants (Linux, zOS and AIX) support all signals that are currently used by the JVM. J9*AsyncHandlerRecord->flags has 32-bits. Currently, 8-bits are unused. So, we can easily extend support for eight other signals. After the 32-bits are used, then a 64-bit data structure would need to be used for flags. This will require a lot of effort since all the API signatures and implementations across all platforms would need to be updated.

The new OMR signal APIs still need to be implemented for ztpf. Based upon previous discussions, ztpf implementation is not needed to close this PR.

Also, I will soon add signal handling tests in OpenJ9.

[pshipton]

The deadline is about Sep 14.

[SueChaplain]

@pshipton The OMR change eclipse/omr#2908 is merged. Presumably there is something else that needs merging to prevents us declaring that signal handling is fixed in 0.10.0? Think it was just Windows that was remaining? Thanks

[pshipton]

@SueChaplain There is some work remaining to support other signals. As macOS is taking priority, I've moved this item to the 0.12.0 milestone.

[pshipton]

@SueChaplain I believe we can remove the Windows limitation from the 0.10.0 release notes. @babsingh please confirm. See the 0.10.0 release notes https://github.com/eclipse/openj9/pull/2766

"Full support for shutdown signals is pending on Windows; signal handlers cannot be registered by using sun.misc.Signal or dk.internal.misc.Signal."

[babsingh]

@SueChaplain @pshipton Peter is correct. Windows has fewer signals. Current implementation supports all signals on Windows. Updated doc:

Currently, shutdown signals (SIGINT, SIGHUP and SIGTERM) and SIGCONT are fully supported on Unix platforms (pLinux, zLinux, xLinux, AIX and zOS). Support for other POSIX signals is pending. See SunMiscSignalTest.java for the list of signals that need to be supported.

[SueChaplain]

@babsingh Thanks. I've updated the release notes for 0.10.0 accordingly.

[pdbain-ibm]

Typo above: "using sun.misc.Signal or dk.internal.misc.Signal." should be jdk?

[pshipton]

@pdbain-ibm that line was removed so the typo is gone. https://github.com/eclipse/openj9/pull/2766

[SueChaplain]

@pshipton - I think I've lost track of what's remaining here. Can you confirm and let me know the release target please.

[pshipton]

@SueChaplain there is no real target to complete this work. I'll put it into the 0.15 milestone so we don't loose track of it, but don't be surprised if it moves out further. The release notes should stay as-is.

[babsingh]

This issue can be closed after https://github.com/eclipse/omr/pull/3921 and https://github.com/eclipse/openj9/pull/5923 are merged. Performing final cross-platform testing.

fyi - @DanHeidinga @pshipton

[pshipton]

@babsingh please create the doc issue to update the release notes, and then we can close this.

[pshipton]

There is a docs issue https://github.com/eclipse/openj9-docs/issues/296, this can be closed.