-
Notifications
You must be signed in to change notification settings - Fork 17.7k
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
os: Process.Signal on the current PID should signal the current thread #19326
Comments
This might be a good idea; I'm not sure. I just want to note that it is possible to write a program today that will work today but would fail with this suggestion. For example, if you block |
That's a good point. Perhaps we could kill the current thread if the signal is unblocked, and the overall process otherwise? Do you have any opinion as to whether that would be better or worse than adding an explicit |
Go programs normally interact with signals via the os/signal package, and of course we don't have to actually raise a signal to pretend that we received one. So the only possible purposes I can see for explicitly raising a signal are
You can get a core dump by calling To invoke a C signal handler or to set the exit status, I don't see why it matters which thread receives the signal. So to me the argument for this does not yet seem compelling. You can always use |
The use-case I have in mind is for invoking C handlers, particularly for fatal signals (such as It's nice to have the signal to arrive at the specific thread because the C handler may itself trigger a core dump (or otherwise capture a traceback of the signaled thread), and keeping the signal on the current thread ensures that that traceback is for the goroutine that actually encountered the problem.
Indeed, although in practice it seems that I have to pass (There are certainly workarounds, but they aren't as portable.) |
If you know for sure there is a C handler, then it would not be unreasonable to use cgo/SWIG to call the C function |
I wonder how the requested behavior matches to the Go runtime assumption of "there are no threads visible to the user"? If this is a feature to improve cooperation with C, it could be implemented in the C part of the hybrid program. Given the above consideration, is this feature about the runtime being aware of the fact that some signals are not intended for the whole process, but individual threads? And then again how does this interact with the runtime assumption of "there are no user visible threads".? So again the receiving part could be implemented in the C part of a CGO enabled program with a Go callback. Now the question (which I didn't research yet) is: Does the Go runtime handle per thread signals well enough, on all platforms that support it, to allow both C helpers to be written? Or is this whole feature intended to only support a dedicated thread reserved via runtime.LockOSThread? |
Agreed. The use-case I have in mind is for reporting a major invariant violation in a Go library that may be used in pure Go programs, mostly-C programs (via cgo export), or anything in between. I do not want the library to add a libc dependency for Go programs that would not otherwise have one. I cannot use |
In a Go program that includes C code, threads are visible to the C portion of the program. The runtime can only assume that threads are not visible to the Go portion of the program. Since a non-blocked signal sent to the process could be delivered to the current thread anyway, making it so that the signal is always sent to the current thread only changes the distribution of observed behaviors — it does not add a new behavior per se.
The Go libraries in use by a program do not, in general, know whether "the C part of the hybrid program" even exists; see my previous comment.
The Go runtime does handle per-thread signals reasonably, or can be made to do so. I do not see any viable way to implement the requested behavior using only C helpers, regardless of the runtime's ability to handle the signals.
An explicit call to Current workarounds do require an explicit call to |
As @ianlancetaylor pointed out, C programs can call C.raise. |
If not, seems like we should close this. |
Yes: for programs that (a) have crash-reporting signal handlers that capture stack traces or core files (such as those used for mobile apps) and (b) exit by raising In the Google-internal bug report I received, by the time the signal arrived, the handler saw |
To be clear: I'm not sure whether the problem is due to asynchronous signal delivery or thread identity. |
remote-apis-testing shows the following log in https://gitlab.com/remote-apis-testing/remote-apis-testing/-/jobs/2773967861 2022/07/26 20:20:14 Received terminated signal. Initiating graceful shutdown. 2022/07/26 20:20:14 Graceful shutdown complete panic: Raising the original signal didn't cause us to shut down Apparently, the panic line was reached before the signal had been handled and the process killed, probably due to a different thread is receiving the signal. For more details, see golang/go#19326
remote-apis-testing shows the following log in https://gitlab.com/remote-apis-testing/remote-apis-testing/-/jobs/2773967861 2022/07/26 20:20:14 Received terminated signal. Initiating graceful shutdown. 2022/07/26 20:20:14 Graceful shutdown complete panic: Raising the original signal didn't cause us to shut down Apparently, the panic line was reached before the signal had been handled and the process killed, probably due to a different thread is receiving the signal. For more details, see golang/go#19326
The Go standard library currently lacks an equivalent to the standard C
raise
function.The closest portable analogue is something like:
Unfortunately, this pattern is somewhat error-prone: on many platforms (e.g. Linux), it can deliver the signal to any thread in the process group. If the purpose of raising the signal is to produce a stack trace or trigger the invocation of a C fatal-signal handler (e.g. by raising
SIGQUIT
orSIGABRT
), that can result in the handler executing on a thread unrelated to the failure.It's tempting to request a new function call,
os.Raise
, for this purpose, but it seems like we could address this use-case automatically without an API change. We could have(*os.Process).Signal
check whether theProcess
in question is the Go program itself and raise the signal synchronously to the current thread (e.g. by using thetgkill
syscall on Linux, or callingpthread_kill
orraise
in cgo-enabled builds).The text was updated successfully, but these errors were encountered: