Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Signal handler installed with signal() is not triggered after GC #137

Open
charleskorn opened this issue Aug 18, 2019 · 3 comments
Open

Comments

@charleskorn
Copy link

charleskorn commented Aug 18, 2019

I've been having trouble with a signal handler installed with POSIX.signal().

It behaves correctly shortly after it is created, but later fails to work. I believe the trigger for it to stop working is a garbage collection run.

I've created a sample application at https://github.com/charleskorn/jnr-signals-issue that installs a signal handler for SIGWINCH and then calls raise(SIGWINCH) to invoke it every second. After five seconds, it calls System.gc() to trigger a garbage collection run. Before the System.gc() call, the signal handler is invoked, and afterwards, it is not.

If I disable the calls to raise and instead use kill -winch <pid> from the command line, the signal handler is successfully invoked before the System.gc() call, but is not once System.gc() is called.

If you run the application (./gradlew run), you'll see output similar to the following:

Raising SIGWINCH...
My PID is 11084
Received it!
Now sent 1, received 1
Sleeping...
Raising SIGWINCH...
Received it!
Now sent 2, received 2
Sleeping...
Raising SIGWINCH...
Received it!
Now sent 3, received 3
Sleeping...
Raising SIGWINCH...
Received it!
Now sent 4, received 4
Sleeping...
Raising SIGWINCH...
Received it!
Now sent 5, received 5
Sleeping...
Invoking GC...
Raising SIGWINCH...
Couldn't raise signal:
java.lang.NullPointerException: callable is null
        at jnr.ffi.provider.jffi.NativeClosureProxy.getCallable(NativeClosureProxy.java:57)
        at jnr.ffi.provider.jffi.NativeClosureProxy$$impl$$0.invoke(Unknown Source)
        at jnr.posix.UnixLibC$jnr$ffi$0.raise(Native Method)
        at jnr.signals.issue.AppKt$main$2.invoke(App.kt:24)
        at jnr.signals.issue.AppKt$main$2.invoke(App.kt)
        at kotlin.concurrent.ThreadsKt$thread$thread$1.run(Thread.kt:30)
Sleeping...
Invoking GC...
Raising SIGWINCH...
Couldn't raise signal:
java.lang.NullPointerException: callable is null
        at jnr.ffi.provider.jffi.NativeClosureProxy.getCallable(NativeClosureProxy.java:57)
        at jnr.ffi.provider.jffi.NativeClosureProxy$$impl$$0.invoke(Unknown Source)
        at jnr.posix.UnixLibC$jnr$ffi$0.raise(Native Method)
Sleeping...
        at jnr.signals.issue.AppKt$main$2.invoke(App.kt:24)
        at jnr.signals.issue.AppKt$main$2.invoke(App.kt)
        at kotlin.concurrent.ThreadsKt$thread$thread$1.run(Thread.kt:30)

Is there something that I'm doing wrong, or is this an issue in this library or another part of JNR?

@charleskorn charleskorn changed the title Signal handler installed with signal is not triggered after GC Signal handler installed with signal() is not triggered after GC Aug 18, 2019
@charleskorn
Copy link
Author

Solved my own problem... looks like the issue was that BaseNativePOSIX's implementation of signal is incorrect. The parameter it passes to LibC's signal is not kept as a reference, so it will be garbage collected (jnr-ffi only keeps a weak reference to closure parameters). Switching to use POSIXFactory.getNativePOSIX().libc().signal(...) worked around this.

An additional complication was that you can't use a Kotlin lambda or method reference as the parameter to signal, as then the Kotlin compiler generates an implementation of the interface for signal implicitly. The implementation of the LibC.LibCSignalHandler interface is the thing that is retained by jnr-ffi with a weak reference, so I need to explicitly implement the interface and then pass a reference to that.

I've updated my sample repo (https://github.com/charleskorn/jnr-signals-issue) with a workaround for these issues if anyone else runs into them.

tl;dr: BaseNativePOSIX.signal() should be retaining a reference to the LibC.LibCSignalHandler it passes to LibC.signal().

@headius
Copy link
Member

headius commented Jan 20, 2020

@charleskorn Is there a jnr-posix patch possible here?

@charleskorn
Copy link
Author

Yes, I think jnr-posix could be patched. The simplest thing would be to remove the distinction between LibCSignalHandler and SignalHandler and just pass the signal handler passed into BaseNativePOSIX.signal() to LibC.signal(), retaining it in signalHandlers as it currently does.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants