Use reflection to install system signal handlers in JVM applications #9251
Conversation
| HANDLE_STATIC_METHOD_HANDLE = initHandleStaticMethodHandle(lookup, signalClass, signalHandlerClass); | ||
| SIGNAL_HANDLER = new SignalHandler.SunMiscSignalHandler(signalHandlerClass); | ||
| } else { | ||
| System.err.println("WARNING: sun.misc.Signal and sun.misc.SignalHandler are not available on this platform. " + |
There was a problem hiding this comment.
Couple of questions here:
- Should we use a logger instead of
System.err.println? - Should we add some option to disable this warning? Might be annoying in some cases, but I think as a default a user should be aware that the fiber dump functionality won't work as expected
There was a problem hiding this comment.
Thanks for the recommendation! I switched to using java.logging to log the warning and changed the code to make it a bit more readable / safe so that we log the warning if any of the reflective methods fail
| } catch (Throwable err) { | ||
| // Gracefully degrade to a no-op logging the exception. | ||
| } |
There was a problem hiding this comment.
Is it safe to catch all Throwable?
Also, is there a reason to use Throwable here and Exception above?
There was a problem hiding this comment.
It's mostly because the invoke method can throw any Throwable (depending on the underlying method) whereas the method above only throw exceptions. I think it's relatively safe to catch all Throwable's here because I don't think we want to crash the application in the very unlikely edge-case that we failed to handle a signal
| * JVMs. | ||
| * </p> | ||
| */ | ||
| final class Signal { |
There was a problem hiding this comment.
Is there a reason to write this in java?
There was a problem hiding this comment.
Actually, not really. I'm not really sure why I thought we'd need Java for this. I rewrote the implementation in Scala
/fixes #9240
NOTE: This approach is a port of the
cats-effectapproach when installing system signal handlers.In short, we use runtime reflection and a proxy class in order to avoid crashing the application with
NoClassDefFoundErrorin cases that the JVM doesn't contain thesun.misc.Signalandsun.misc.SignalHandlerclasses. It also adds config to include these classes when compiling the code using GraalVM's native image.I added a comment / question below, please let me know what you think