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

runtime: dieFromSignal should reinstall fwdSig[sig] instead of hard-coding _SIG_DFL #19389

Closed
bcmills opened this issue Mar 3, 2017 · 3 comments
Milestone

Comments

@bcmills
Copy link
Contributor

bcmills commented Mar 3, 2017

runtime.dieFromSignal is called in a few places in the runtime where we've handled a fatal signal and intend to terminate the process.

At the moment, dieFromSignal unconditionally calls setsig(sig, _SIG_DFL) and re-raises the signal.
That's pretty much always the right thing to do in a pure-Go program.

However, in a mixed-language program there may be some existing signal handler, and that signal handler might want to do something useful — for example, save a core dump, mark an input record as a potential cause of the crash, or write an entry to a crash log.

It seems to me that we could address this use-case pretty easily by making dieFromSignal try to reinstall and raise to the previous handler before it resorts to SIG_DFL. Perhaps something like:

func dieFromSignal(sig uint32) {
	fwdFn := atomic.Loaduintptr(&fwdSig[sig])
	if fwdFn != _SIG_DFL {
		setsig(sig, fwdFn)
		unblocksig(sig)
		raise(sig)
	}

	// fwdSig[sig] either didn't exist or failed to terminate the process.
	// Try _SIG_DFL instead.
	setsig(sig, _SIG_DFL)
	unblocksig(sig)
	raise(sig)

	// That should have killed us. On some systems, though, raise
	// sends the signal to the whole process rather than to just
	// the current thread, which means that the signal may not yet
	// have been delivered. Give other threads a chance to run and
	// pick up the signal.
	osyield()
	osyield()
	osyield()

	// If we are still somehow running, just exit with the wrong status.
	exit(2)
}

In the typical case — when there are no handlers registered before ours — the behavior would be the same as it is today. On the off chance that some other language has registered a prior handler, we would instead invoke that handler, and if it isn't well behaved (i.e. doesn't terminate the process), then we fall back to today's behavior anyway.

@ianlancetaylor
Copy link
Contributor

SGTM

It's only necessary to call unblockSig once.

@gopherbot
Copy link

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

@bradfitz bradfitz modified the milestones: Go1.9Maybe, Go1.10 Jul 20, 2017
@gopherbot
Copy link

Change https://golang.org/cl/55032 mentions this issue: runtime: fix crashing with foreign signal handlers on Darwin

gopherbot pushed a commit that referenced this issue Aug 11, 2017
The dieFromSignal runtime function attempts to forward crashing
signals to a signal handler registered before the runtime was
initialized, if any. However, on Darwin, a special signal handler
trampoline is invoked, even for non-Go signal handlers.

Clear the crashing signal's handlingSig entry to ensure sigtramp
forwards the signal.

Fixes the darwin/386 builder.

Updates #20392
Updates #19389

Change-Id: I441a3d30c672cdb21ed6d8f1e1322d7c0e5b9669
Reviewed-on: https://go-review.googlesource.com/55032
Run-TryBot: Elias Naur <elias.naur@gmail.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
@golang golang locked and limited conversation to collaborators Aug 11, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants