Commit f41bf587 by Ian Lance Taylor

runtime: dropg before CAS g status to _Grunnable/_Gwaiting

    
    Currently, we dropg (which clears gp.m) after we CAS the g status
    to _Grunnable or _Gwaiting. Immediately after CASing the g status,
    another thread may CAS it to _Gscan status and scan its stack.
    With precise stack scan, it accesses gp.m in order to switch to g
    and back (in doscanstackswitch). This races with dropg. If
    doscanstackswitch reads gp.m, then dropg runs, when we restore
    the m at the end of the scan it will set to a stale value. Worse,
    if dropg runs after doscanstackswitch sets the new m, gp will be
    running with a nil m.
    
    To fix this, we do dropg before CAS g status to _Grunnable or
    _Gwaiting. We can do this safely if we are CASing from _Grunning,
    as we own the g when it is in _Grunning. There is one case where
    we CAS from _Gsyscall to _Grunnable. It is not safe to dropg when
    it is in _Gsyscall, as precise stack scan needs to read gp.m in
    order to signal the m. So we need to introduce a transient state,
    _Gexitingsyscall, between _Gsyscall and _Grunnable, where the GC
    should not scan its stack.
    
    In is a little unfortunate that we have to add another g status.
    We could reuse an existing one (e.g. _Gcopystack), but it is
    clearer and safer to just use a new one, as Austin suggested.
    
    Reviewed-on: https://go-review.googlesource.com/c/158157

From-SVN: r268001
parent 63dfd55e
ee94431c133a90ca5c3c5ebbebcb019c60258dac d6576c83016d856217758c06d945bfc363ffb817
The first line of this file holds the git revision number of the last The first line of this file holds the git revision number of the last
merge done from the gofrontend repository. merge done from the gofrontend repository.
...@@ -956,6 +956,10 @@ loop: ...@@ -956,6 +956,10 @@ loop:
break loop break loop
} }
case _Gexitingsyscall:
// This is a transient state during which we should not scan its stack.
// Try again.
case _Gscanwaiting: case _Gscanwaiting:
// newstack is doing a scan for us right now. Wait. // newstack is doing a scan for us right now. Wait.
...@@ -2635,8 +2639,8 @@ func park_m(gp *g) { ...@@ -2635,8 +2639,8 @@ func park_m(gp *g) {
traceGoPark(_g_.m.waittraceev, _g_.m.waittraceskip) traceGoPark(_g_.m.waittraceev, _g_.m.waittraceskip)
} }
casgstatus(gp, _Grunning, _Gwaiting)
dropg() dropg()
casgstatus(gp, _Grunning, _Gwaiting)
if _g_.m.waitunlockf != nil { if _g_.m.waitunlockf != nil {
fn := *(*func(*g, unsafe.Pointer) bool)(unsafe.Pointer(&_g_.m.waitunlockf)) fn := *(*func(*g, unsafe.Pointer) bool)(unsafe.Pointer(&_g_.m.waitunlockf))
...@@ -2660,8 +2664,8 @@ func goschedImpl(gp *g) { ...@@ -2660,8 +2664,8 @@ func goschedImpl(gp *g) {
dumpgstatus(gp) dumpgstatus(gp)
throw("bad g status") throw("bad g status")
} }
casgstatus(gp, _Grunning, _Grunnable)
dropg() dropg()
casgstatus(gp, _Grunning, _Grunnable)
lock(&sched.lock) lock(&sched.lock)
globrunqput(gp) globrunqput(gp)
unlock(&sched.lock) unlock(&sched.lock)
...@@ -3054,8 +3058,9 @@ func exitsyscallfast_pidle() bool { ...@@ -3054,8 +3058,9 @@ func exitsyscallfast_pidle() bool {
func exitsyscall0(gp *g) { func exitsyscall0(gp *g) {
_g_ := getg() _g_ := getg()
casgstatus(gp, _Gsyscall, _Grunnable) casgstatus(gp, _Gsyscall, _Gexitingsyscall)
dropg() dropg()
casgstatus(gp, _Gexitingsyscall, _Grunnable)
lock(&sched.lock) lock(&sched.lock)
_p_ := pidleget() _p_ := pidleget()
if _p_ == nil { if _p_ == nil {
......
...@@ -70,6 +70,12 @@ const ( ...@@ -70,6 +70,12 @@ const (
// stack is owned by the goroutine that put it in _Gcopystack. // stack is owned by the goroutine that put it in _Gcopystack.
_Gcopystack // 8 _Gcopystack // 8
// _Gexitingsyscall means this goroutine is exiting from a
// system call. This is like _Gsyscall, but the GC should not
// scan its stack. Currently this is only used in exitsyscall0
// as a transient state when it drops the G.
_Gexitingsyscall // 9
// _Gscan combined with one of the above states other than // _Gscan combined with one of the above states other than
// _Grunning indicates that GC is scanning the stack. The // _Grunning indicates that GC is scanning the stack. The
// goroutine is not executing user code and the stack is owned // goroutine is not executing user code and the stack is owned
......
...@@ -122,13 +122,14 @@ func isExportedRuntime(name string) bool { ...@@ -122,13 +122,14 @@ func isExportedRuntime(name string) bool {
} }
var gStatusStrings = [...]string{ var gStatusStrings = [...]string{
_Gidle: "idle", _Gidle: "idle",
_Grunnable: "runnable", _Grunnable: "runnable",
_Grunning: "running", _Grunning: "running",
_Gsyscall: "syscall", _Gsyscall: "syscall",
_Gwaiting: "waiting", _Gwaiting: "waiting",
_Gdead: "dead", _Gdead: "dead",
_Gcopystack: "copystack", _Gcopystack: "copystack",
_Gexitingsyscall: "exiting syscall",
} }
func goroutineheader(gp *g) { func goroutineheader(gp *g) {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment