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

Comments

Projects
None yet
4 participants
@bcmills
Copy link
Member

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

This comment has been minimized.

Copy link
Contributor

commented Mar 4, 2017

SGTM

It's only necessary to call unblockSig once.

@gopherbot

This comment has been minimized.

Copy link

commented Jul 18, 2017

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

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

@gopherbot gopherbot closed this in 5500c9c Aug 10, 2017

@gopherbot

This comment has been minimized.

Copy link

commented Aug 11, 2017

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

Elias Naur
runtime: fix crashing with foreign signal handlers on Darwin
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.
You can’t perform that action at this time.