««« CL 179680043 / 752cd9199639
runtime: fix hang in GC due to shrinkstack vs netpoll race
During garbage collection, after scanning a stack, we think about
shrinking it to reclaim some memory. The shrinking code (called
while the world is stopped) checked that the status was Gwaiting
or Grunnable and then changed the state to Gcopystack, to essentially
lock the stack so that no other GC thread is scanning it.
The same locking happens for stack growth (and is more necessary there).
oldstatus = runtime·readgstatus(gp);
oldstatus &= ~Gscan;
if(oldstatus == Gwaiting || oldstatus == Grunnable)
runtime·casgstatus(gp, oldstatus, Gcopystack); // oldstatus is Gwaiting or Grunnable
else
runtime·throw("copystack: bad status, not Gwaiting or Grunnable");
Unfortunately, "stop the world" doesn't stop everything. It stops all
normal goroutine execution, but the network polling thread is still
blocked in epoll and may wake up. If it does, and it chooses a goroutine
to mark runnable, and that goroutine is the one whose stack is shrinking,
then it can happen that between readgstatus and casgstatus, the status
changes from Gwaiting to Grunnable.
casgstatus assumes that if the status is not what is expected, it is a
transient change (like from Gwaiting to Gscanwaiting and back, or like
from Gwaiting to Gcopystack and back), and it loops until the status
has been restored to the expected value. In this case, the status has
changed semi-permanently from Gwaiting to Grunnable - it won't
change again until the GC is done and the world can continue, but the
GC is waiting for the status to change back. This wedges the program.
To fix, call a special variant of casgstatus that accepts either Gwaiting
or Grunnable as valid statuses.
Without the fix bug with the extra check+throw in casgstatus, the
program below dies in a few seconds (2-10) with GOMAXPROCS=8
on a 2012 Retina MacBook Pro. With the fix, it runs for minutes
and minutes.
package main
import (
"io"
"log"
"net"
"runtime"
)
func main() {
const N = 100
for i := 0; i < N; i++ {
l, err := net.Listen("tcp", "127.0.0.1:0")
if err != nil {
log.Fatal(err)
}
ch := make(chan net.Conn, 1)
go func() {
var err error
c1, err := net.Dial("tcp", l.Addr().String())
if err != nil {
log.Fatal(err)
}
ch <- c1
}()
c2, err := l.Accept()
if err != nil {
log.Fatal(err)
}
c1 := <-ch
l.Close()
go netguy(c1, c2)
go netguy(c2, c1)
c1.Write(make([]byte, 100))
}
for {
runtime.GC()
}
}
func netguy(r, w net.Conn) {
buf := make([]byte, 100)
for {
bigstack(1000)
_, err := io.ReadFull(r, buf)
if err != nil {
log.Fatal(err)
}
w.Write(buf)
}
}
var g int
func bigstack(n int) {
var buf [100]byte
if n > 0 {
bigstack(n - 1)
}
g = int(buf[0]) + int(buf[99])
}
Fixes#9186.
LGTM=rlh
R=austin, rlh
CC=dvyukov, golang-codereviews, iant, khr, r
https://golang.org/cl/179680043
»»»
TBR=rlh
CC=golang-codereviews
https://golang.org/cl/184030043
««« CL 179030043 / e4ab8f908aac
runtime: fix atomic operations on non-heap addresses
Race detector runtime does not tolerate operations on addresses
that was not previously declared with __tsan_map_shadow
(namely, data, bss and heap). The corresponding address
checks for atomic operations were removed in
https://golang.org/cl/111310044
Restore these checks.
It's tricker than just not calling into race runtime,
because it is the race runtime that makes the atomic
operations themselves (if we do not call into race runtime
we skip the atomic operation itself as well). So instead we call
__tsan_go_ignore_sync_start/end around the atomic operation.
This forces race runtime to skip all other processing
except than doing the atomic operation itself.
Fixes#9136.
LGTM=rsc
R=rsc
CC=golang-codereviews
https://golang.org/cl/179030043
»»»
TBR=dvyukov
CC=golang-codereviews
https://golang.org/cl/180030043
««« CL 176070043 / 500cb52e08e6
build: disable race external linking test on OS X 10.6 and earlier
External linking doesn't work there at all.
LGTM=bradfitz
R=adg, bradfitz
CC=golang-codereviews
https://golang.org/cl/176070043
»»»
LGTM=bradfitz, adg
R=adg, bradfitz
CC=golang-codereviews
https://golang.org/cl/175400043
««« CL 179980043 / d71cc7e8a0e0
runtime: remove assumption that noptrdata data bss noptrbss are ordered and contiguous
The assumption can be violated by external linkers reordering them or
inserting non-Go sections in between them. I looked briefly at trying
to write out the _go_.o in external linking mode in a way that forced
the ordering, but no matter what there's no way to force Go's data
and Go's bss to be next to each other. If there is any data or bss from
non-Go objects, it's very likely to get stuck in between them.
Instead, rewrite the two places we know about that make the assumption.
I grepped for noptrdata to look for more and didn't find any.
The added race test (os/exec in external linking mode) fails without
the changes in the runtime. It crashes with an invalid pointer dereference.
Fixes#9133.
LGTM=dneil
R=dneil
CC=dvyukov, golang-codereviews, iant
https://golang.org/cl/179980043
»»»
LGTM=dneil
R=dneil
CC=golang-codereviews
https://golang.org/cl/173510043
««« CL 174450043 / 699cc091a16d
undo CL 131750044 / 2d6d44ceb80e
Breaks reading from stdin in parent after exec with SysProcAttr{Setpgid: true}.
package main
import (
"fmt"
"os"
"os/exec"
"syscall"
)
func main() {
cmd := exec.Command("true")
cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true}
cmd.Run()
fmt.Printf("Hit enter:")
os.Stdin.Read(make([]byte, 100))
fmt.Printf("Bye\n")
}
In go1.3, I type enter at the prompt and the program exits.
With the CL being rolled back, the program wedges at the
prompt.
««« original CL description
syscall: SysProcAttr job control changes
Making the child's process group the foreground process group and
placing the child in a specific process group involves co-ordination
between the parent and child that must be done post-fork but pre-exec.
LGTM=iant
R=golang-codereviews, gobot, iant, mikioh.mikioh
CC=golang-codereviews
https://golang.org/cl/131750044
»»»
LGTM=minux, dneil
R=dneil, minux
CC=golang-codereviews, iant, michael.p.macinnis
https://golang.org/cl/174450043
»»»
LGTM=minux
R=dneil, minux
CC=golang-codereviews
https://golang.org/cl/179970043
««« CL 174250043 / c16349455e05
debug/goobj: move to cmd/internal/goobj
debug/goobj is not ready to be published but it is
needed for the various binary-reading commands.
Move to cmd/internal/goobj.
(The Go 1.3 release branch deleted it, but that's not
an option anymore due to the command dependencies.
The API is still not vetted nor terribly well designed.)
LGTM=adg, dsymonds
R=adg, dsymonds
CC=golang-codereviews
https://golang.org/cl/174250043
»»»
LGTM=rsc
R=rsc
CC=golang-codereviews
https://golang.org/cl/177890043
The SudoG used to sit on the stack, so it was cheap to allocated
and didn't need to be cleaned up when finished.
For the conversion to Go, we had to move sudog off the stack
for a few reasons, so we added a cache of recently used sudogs
to keep allocation cheap. But we didn't add any of the necessary
cleanup before adding a SudoG to the new cache, and so the cached
SudoGs had stale pointers inside them that have caused all sorts
of awful, hard to debug problems.
CL 155760043 made sure SudoG.elem is cleaned up.
CL 150520043 made sure SudoG.selectdone is cleaned up.
This CL makes sure SudoG.next, SudoG.prev, and SudoG.waitlink
are cleaned up. I should have done this when I did the other two
fields; instead I wasted a week tracking down a leak they caused.
A dangling SudoG.waitlink can point into a sudogcache list that
has been "forgotten" in order to let the GC collect it, but that
dangling .waitlink keeps the list from being collected.
And then the list holding the SudoG with the dangling waitlink
can find itself in the same situation, and so on. We end up
with lists of lists of unusable SudoGs that are still linked into
the object graph and never collected (given the right mix of
non-trivial selects and non-channel synchronization).
More details in golang.org/issue/9110.
Fixes#9110.
LGTM=r
R=r
CC=dvyukov, golang-codereviews, iant, khr
https://golang.org/cl/177870043
I just created that redirect, so we can change
it once the wiki moves.
LGTM=bradfitz, khr
R=khr, bradfitz
CC=golang-codereviews
https://golang.org/cl/177780043
Per private thread soliciting help. I realized part of this is
documented in several places, but we lacked a unifying
example.
LGTM=rsc
R=golang-codereviews
CC=adg, golang-codereviews, iant, rsc
https://golang.org/cl/171620043
This patch is based only on reading the code. I have not
tried to construct a test case.
Fixes#9077.
LGTM=minux
R=minux
CC=golang-codereviews
https://golang.org/cl/172110043
Manifested as increased memory usage in a Google production system.
Not an unbounded leak, but can significantly increase the number
of sudogs allocated between garbage collections.
I checked all the other calls to acquireSudog.
This is the only one that was missing a releaseSudog.
LGTM=r, dneil
R=dneil, r
CC=golang-codereviews
https://golang.org/cl/169260043
These are being built into the runtime/cgo for every
operating system. It doesn't seem to matter, but
restore the Go 1.3 behavior anyway.
LGTM=r
R=r, dave
CC=golang-codereviews
https://golang.org/cl/171290043
This was a mistake. The cmd/api tool
depends on an old version of go/types.
««« original CL description
cmd/api: use golang.org/x/... import paths
LGTM=bradfitz, rsc
R=rsc, bradfitz
CC=golang-codereviews
https://golang.org/cl/169000043
»»»
TBR=rsc, bradfitz
R=bradfitz, rsc
CC=golang-codereviews
https://golang.org/cl/169320043
This was a mistake; the cmd/api tool
depends on an old version of go/types.
««« original CL description
cmd/api: bump go.tools golden CL hash
TBR=bradfitz
R=rsc
CC=golang-codereviews
https://golang.org/cl/166380043
»»»
TBR=bradfitz, rsc
R=bradfitz, rsc
CC=golang-codereviews
https://golang.org/cl/167430043
New detection because of net/http now using TestMain.
Fixes#9033
LGTM=iant
R=golang-codereviews, iant
CC=adg, golang-codereviews, rsc
https://golang.org/cl/170210043
This was missing from CL 167320043.
Happy to apply comments in a followup.
TBR to fix build.
TBR=r
CC=golang-codereviews
https://golang.org/cl/171260043
Moving so that new Go 1.4 pprof can use it.
The old 'GNU objdump workalike' mode for 'go tool objdump'
is now gone, as are the tests for that mode. It was used only
by pre-Go 1.4 pprof. You can still specify an address range on
the command line; you just get the same output format as
you do when dumping the entire binary (without an address
limitation).
LGTM=r
R=r
CC=golang-codereviews, iant
https://golang.org/cl/167320043
Stack bitmaps need to be scanned past any BitsDead entries.
Object bitmaps will not have any BitsDead in them (bitmap extraction stops at
the first BitsDead entry in makeheapobjbv). data/bss bitmaps also have no BitsDead entries.
LGTM=rsc
R=rsc
CC=golang-codereviews
https://golang.org/cl/168270043
For some reason lsof is now hanging on my workstation
without the -b (avoid blocking in the kernel) option.
Adding -b makes the test pass and shouldn't hurt.
I don't know how recent the -b option is. If the builders
are ok with it, it's probably ok.
LGTM=rsc
R=golang-codereviews, bradfitz, rsc
CC=golang-codereviews
https://golang.org/cl/166220043
Gentraceback may grow the stack.
One of the gentraceback wrappers may grow the stack.
One of the gentraceback callback calls may grow the stack.
Various stack pointers are stored in various stack locations
as type uintptr during the execution of these calls.
If the stack does grow, these stack pointers will not be
updated and will start trying to decode stack memory that
is no longer valid.
It may be possible to change the type of the stack pointer
variables to be unsafe.Pointer, but that's pretty subtle and
may still have problems, even if we catch every last one.
An easier, more obviously correct fix is to require that
gentraceback of the currently running goroutine must run
on the g0 stack, not on the goroutine's own stack.
Not doing this causes faults when you set
StackFromSystem = 1
StackFaultOnFree = 1
The new check in gentraceback will catch future lapses.
The more general problem is calling getcallersp but then
calling a function that might relocate the stack, which would
invalidate the result of getcallersp. Add note to stubs.go
declaration of getcallersp explaining the problem, and
check all existing calls to getcallersp. Most needed fixes.
This affects Callers, Stack, and nearly all the runtime
profiling routines. It does not affect stack copying directly
nor garbage collection.
LGTM=khr
R=khr, bradfitz
CC=golang-codereviews, r
https://golang.org/cl/167060043
The new rules for split functions mean that we are exposed
to the common bug of a function that loops forever at EOF.
Pick these off by shutting down the scanner if too many
consecutive empty tokens are delivered.
Fixes#9020.
LGTM=rsc, adg
R=golang-codereviews, rsc, adg, bradfitz
CC=golang-codereviews
https://golang.org/cl/169970043
The test intended to skip direct calls when creating
registerization variables was testing p->to.type instead of
p->to.name, so it always failed, causing regopt to create
unnecessary variables for these names.
LGTM=rsc
R=rsc
CC=golang-codereviews
https://golang.org/cl/169110043
The check for unknown command line debug flags in gc was
incorrect: the loop over debugtab terminates when it reaches a
nil entry, but it was only reporting an error if the parser
had passed the last entry of debugtab (which it never did).
Fix this by reporting the usage error if the loop reaches a
nil entry.
LGTM=rsc
R=rsc
CC=golang-codereviews
https://golang.org/cl/166110043
Previously, the flags argument to mallocgc was an int in Go,
but a uint32 in C. Change the Go type to use uint32 so these
agree. The largest flag value is 2 (and of course no flag
values are negative), so this won't change anything on little
endian architectures, but it matters on big endian.
LGTM=rsc
R=khr, rsc
CC=golang-codereviews
https://golang.org/cl/169920043
Russ Cox
Andrew Gerrand
Nigel Tao
Brad Fitzpatrick
Robin Eklind
Ian Lance Taylor
Keith Randall
Rob Pike
Austin Clements