Skip to content

runtime: -buildmode=c-shared generated code hijack's signals from main program #11794

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.

Sign up for GitHub

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

Jump to bottom
Closed
GeertJohan opened this issue Jul 19, 2015 · 10 comments
Closed

runtime: -buildmode=c-shared generated code hijack's signals from main program #11794

GeertJohan opened this issue Jul 19, 2015 · 10 comments
Labels
FrozenDueToAge
Milestone
Go1.5

Comments

@GeertJohan
Copy link
Contributor

It looks like a shared object created with -buildmode=c-shared "hijack's" signal handling away from the main program.

I'm experimenting with Go shared objects as plugins. The main process has signal handling implemented to do graceful shutdown. However, the main program does not receive signals anymore after a Go shared object was loaded.

I would expect the main program to receive (and handle) the signals. Instead, signals are completely ignored. It requires SIGKILL to kill the process.

Simple code to reproduce:

main.go:

package main

func init() {}

func main() {}

_main.c:

#include <stdlib.h>
#include <stdio.h>
#include <dlfcn.h>

int main() {
    void *plugin;
    plugin = dlopen("./gopkg.so", RTLD_NOW);
    if (!plugin) {
         exit(1);
    }

    while(1) {
        sleep(1);
    }

    return 0;
}

run:

go build -buildmode c-shared -o gopkg.so
gcc -o main _main.c -ldl
./main
ctrl-c
@GeertJohan GeertJohan changed the title c-shared hijack's signals from main program -buildmode=c-shared generated code hijack's signals from main program Jul 19, 2015
@mikioh mikioh changed the title -buildmode=c-shared generated code hijack's signals from main program runtime: -buildmode=c-shared generated code hijack's signals from main program Jul 19, 2015
@GeertJohan
Copy link
Contributor Author

I tried this with go1.5beta2 as well as the latest master at 1942e38

@ianlancetaylor ianlancetaylor added this to the Go1.6 milestone Jul 20, 2015
@ianlancetaylor ianlancetaylor self-assigned this Jul 20, 2015
@ianlancetaylor
Copy link
Member

This is clearly wrong, but I think the code is subtle enough that we won't be able to fix it for 1.5.

@gopherbot
Copy link
Contributor

CL https://golang.org/cl/12503 mentions this issue.

@GeertJohan
Copy link
Contributor Author

It looks like cl 12503 fixes this partly. The signal isn't ignored anymore, but the original signal handler doesn't handle the signal either.

What do you mean with "code is subtle enough"?
Thanks!

@ianlancetaylor
Copy link
Member

Why doesn't the original signal handler get the signal? I thought this problem only arose when there was no original signal handler. Do you have a test case?

This is exactly the kind of thing I mean when I say "the code is subtle enough." The purpose of the CL is to fix issue #10139 which is a problem for regular Go code (not using c-shared). If it fixes this issue too that is good, but it's not the goal of the CL.

@GeertJohan
Copy link
Contributor Author

I'm sorry, when I wrote the issue I hadn't read up as much on signal handling as I have now.

So there are two cases I tested. The first one is when there is a custom signal handler set, the second case is when there is no custom handler set (as in the example in this issue).

It looks like the CL fixes the second case; SIGINT now causes the program to stop. It's not ignored anymore.

I don't have a good example for the first case.
In the Aerospike database there is a custom signal handler that starts the database shutdown on SIGINT. (it's implemented in their signal.c) After loading a Go1.5beta2 c-shared .so into the database server, using dlopen, the signal is completely ignored. I assume because the signal handler is overwritten by go. When a c-shared .so built with your CL is loaded, the program does not ignore the SIGINT (jay!), but it also doesn't fire the custom signal handler that was set by Aerospike. Instead the program exits directly, without proper shutdown, with the message received signal interrupt.

ianlancetaylor added a commit that referenced this issue Jul 22, 2015
@ianlancetaylor
runtime: if we don't handle a signal on a non-Go thread, raise it
872b168 
In the past badsignal would crash the program.  In
https://golang.org/cl/10757044 badsignal was changed to call sigsend,
to fix issue #3250.  The effect of this was that when a non-Go thread
received a signal, and os/signal.Notify was not being used to check
for occurrences of the signal, the signal was ignored.

This changes the code so that if os/signal.Notify is not being used,
then the signal handler is reset to what it was, and the signal is
raised again.  This lets non-Go threads handle the signal as they
wish.  In particular, it means that a segmentation violation in a
non-Go thread will ordinarily crash the process, as it should.

Fixes #10139.
Update #11794.

Change-Id: I2109444aaada9d963ad03b1d071ec667760515e5
Reviewed-on: https://go-review.googlesource.com/12503
Reviewed-by: Russ Cox <rsc@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
@ianlancetaylor
Copy link
Member

@GeertJohan I can't recreate the problem. Your original test case works with current tip, as I think we agree. When I try, with current tip, a small modification of your original test case that registers a signal handler for SIGINT, I do see that signal handler installed by C being invoked when I type ^C on the keyboard.

Can you construct a stand-alone test case that shows the problem?

@GeertJohan
Copy link
Contributor Author

@ianlancetaylor Yes, I'll try to do that.

@GeertJohan
Copy link
Contributor Author

It looks like the problem has been completely solved. I can't recreate it anymore with current tip (go version devel +eb248c4).
Thanks @ianlancetaylor !

@ianlancetaylor
Copy link
Member

Great, thanks for testing it.

@mikioh mikioh modified the milestones: Go1.5, Go1.6 Jul 24, 2015
@glycerine glycerine mentioned this issue Oct 23, 2015
Closed
@golang golang locked and limited conversation to collaborators Aug 5, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
Go1.5
Development

No branches or pull requests
4 participants
@mikioh @GeertJohan @ianlancetaylor @gopherbot