- 01 Dec, 2014 2 commits
-
-
Russ Cox authored
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
-
Keith Randall authored
pointer, not one. Fixes #9179 LGTM=iant, rsc R=golang-codereviews, iant, rsc CC=golang-codereviews https://golang.org/cl/182160043
-
- 25 Nov, 2014 2 commits
-
-
Andrew Gerrand authored
LGTM=r R=r CC=golang-codereviews https://golang.org/cl/182750043
-
Russ Cox authored
We decided to build $GOOS.go always but forgot to test $GOOS_test.go. Fixes #9159. LGTM=r R=r CC=golang-codereviews https://golang.org/cl/176290043
-
- 22 Nov, 2014 2 commits
-
-
Russ Cox authored
Fixes #9127. LGTM=r R=bradfitz, r CC=golang-codereviews, nigeltao https://golang.org/cl/178120043
-
Shenghou Ma authored
Fixes #9149. LGTM=alex.brainman, rsc R=rsc, dave, alex.brainman CC=golang-codereviews https://golang.org/cl/176170043
-
- 20 Nov, 2014 3 commits
-
-
Robert Griesemer authored
Only affects test code. Fixes #9025. Fixes #9130. LGTM=r, adonovan R=adonovan, r CC=golang-codereviews https://golang.org/cl/180920043
-
Dmitriy Vyukov authored
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
-
Russ Cox authored
External linking doesn't work there at all. LGTM=bradfitz R=adg, bradfitz CC=golang-codereviews https://golang.org/cl/176070043
-
- 19 Nov, 2014 2 commits
-
-
Russ Cox authored
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
-
Russ Cox authored
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
-
- 18 Nov, 2014 1 commit
-
-
Rob Pike authored
The grammar was atrocious, probably the victim of an editing error. LGTM=bradfitz R=bradfitz CC=golang-codereviews https://golang.org/cl/178910043
-
- 17 Nov, 2014 8 commits
-
-
Alex Brainman authored
LGTM=bradfitz R=rsc, bradfitz CC=golang-codereviews https://golang.org/cl/180760043
-
Austin Clements authored
getFunctionSource gathers five lines of "margin" around every requested sample line. However, if this margin went past the end of the source file, getFunctionSource would encounter an io.EOF error and abort with this error, resulting in listings like (pprof) list main.main ROUTINE ======================== main.main in ... 0 8.33s (flat, cum) 99.17% of Total Error: EOF (pprof) Modify the error handling in getFunctionSource so io.EOF is always considered non-fatal. If it reaches EOF, it simply returns the lines it has. LGTM=bradfitz R=rsc, bradfitz CC=golang-codereviews https://golang.org/cl/172600043
-
Andrew Gerrand authored
Now that the build and builders are fixed, we're good to go. LGTM=dsymonds R=rsc, dsymonds CC=golang-codereviews https://golang.org/cl/177900043
-
David Symonds authored
Turns out it *is* needed because the cmd/link tests expect to find their own files. ««« original CL description misc/nacl: exclude cmd/link from the test zip. It does not appear to be necessary, and cmd/link does not appear in release branches. LGTM=rsc R=adg, rsc CC=golang-codereviews https://golang.org/cl/176900044 »»» TBR=rsc R=adg, rsc CC=golang-codereviews https://golang.org/cl/175870045
-
David Symonds authored
It does not appear to be necessary, and cmd/link does not appear in release branches. LGTM=rsc R=adg, rsc CC=golang-codereviews https://golang.org/cl/176900044
-
Russ Cox authored
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
-
Andrew Gerrand authored
TBR=rsc R=rsc CC=golang-codereviews https://golang.org/cl/175870043
-
Andrew Gerrand authored
LGTM=dsymonds R=rsc, dsymonds CC=golang-codereviews https://golang.org/cl/175840043
-
- 16 Nov, 2014 2 commits
-
-
Russ Cox authored
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
-
Russ Cox authored
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
-
- 14 Nov, 2014 3 commits
-
-
Nigel Tao authored
LGTM=dsymonds R=dsymonds CC=golang-codereviews, nmvc https://golang.org/cl/169580043
-
Yasuhiro Matsumoto authored
LGTM=adg R=golang-codereviews, adg CC=golang-codereviews https://golang.org/cl/170660043
-
Nigel Tao authored
LGTM=bradfitz R=bradfitz, alex.brainman CC=golang-codereviews https://golang.org/cl/168600044
-
- 12 Nov, 2014 4 commits
-
-
Brad Fitzpatrick authored
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
-
Emil Hessman authored
LGTM=bradfitz R=golang-codereviews, bradfitz CC=golang-codereviews https://golang.org/cl/143470043
-
Nigel Tao authored
LGTM=minux R=adg, minux CC=golang-codereviews https://golang.org/cl/170520043
-
Robin Eklind authored
LGTM=iant R=golang-codereviews, iant CC=golang-codereviews https://golang.org/cl/171560043
-
- 11 Nov, 2014 3 commits
-
-
Robert Griesemer authored
Language clarification. The existing rules for selector expressions imply automatic dereferencing of pointers to struct fields. They also implied automatic dereferencing of selectors denoting methods. In almost all cases, such automatic dereferencing does indeed take place for methods but the reason is not the selector rules but the fact that method sets include both methods with T and *T receivers; so for a *T actual receiver, a method expecting a formal T receiver, also accepts a *T (and the invocation or method value expression is the reason for the auto-derefering). However, the rules as stated so far implied that even in case of a variable p of named pointer type P, a selector expression p.f would always be shorthand for (*p).f. This is true for field selectors f, but cannot be true for method selectors since a named pointer type always has an empty method set. Named pointer types may never appear as anonymous field types (and method receivers, for that matter), so this only applies to variables declared of a named pointer type. This is exceedingly rare and perhaps shouldn't be permitted in the first place (but we cannot change that). Amended the selector rules to make auto-deref of values of named pointer types an exception to the general rules and added corresponding examples with explanations. Both gc and gccgo have a bug where they do auto-deref pointers of named types in method selectors where they should not: See http://play.golang.org/p/c6VhjcIVdM , line 45. Fixes #5769. Fixes #8989. LGTM=r, rsc R=r, rsc, iant, ken CC=golang-codereviews https://golang.org/cl/168790043
-
Rob Pike authored
LGTM=adg R=golang-codereviews, adg CC=golang-codereviews https://golang.org/cl/172980043
-
Nigel Tao authored
LGTM=r R=r CC=golang-codereviews https://golang.org/cl/173920043
-
- 10 Nov, 2014 8 commits
-
-
Ian Lance Taylor authored
LGTM=rsc R=rsc CC=golang-codereviews https://golang.org/cl/171360043
-
Ian Lance Taylor authored
Fixes #9078. LGTM=adg R=golang-codereviews, adg CC=golang-codereviews https://golang.org/cl/172920043
-
Ian Lance Taylor authored
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
-
Russ Cox authored
Disable linkx_run.go and sinit_run.go, because they exec subprocesses, which NaCl cannot. TBR=r CC=golang-codereviews https://golang.org/cl/171350043
-
Russ Cox authored
Follow-up in response to comments on TBR'ed CL 171260043. LGTM=r R=r CC=golang-codereviews https://golang.org/cl/172080043
-
Russ Cox authored
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
-
Russ Cox authored
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
-
Russ Cox authored
LGTM=dave, bradfitz, r, alex.brainman R=r, dave, bradfitz, alex.brainman CC=golang-codereviews https://golang.org/cl/167350043
-