1. 14 Jan, 2017 3 commits
  2. 13 Jan, 2017 6 commits
    • Brad Fitzpatrick's avatar
      net/http: make sure Hijack's bufio.Reader includes pre-read background byte · b2a3b54b
      Brad Fitzpatrick authored
      Previously, if the Hijack called stopped the background read call
      which read a byte, that byte was sitting in memory, buffered, ready to
      be Read by Hijack's returned bufio.Reader, but it wasn't yet in the
      bufio.Reader's buffer itself, so bufio.Reader.Buffered() reported 1
      byte fewer.
      
      This matters for callers who wanted to stitch together any buffered
      data (with bufio.Reader.Peek(bufio.Reader.Buffered())) with Hijack's
      returned net.Conn. Otherwise there was no way for callers to know a
      byte was read.
      
      Change-Id: Id7cb0a0a33fe2f33d79250e13dbaa9c0f7abba13
      Reviewed-on: https://go-review.googlesource.com/35232
      Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarJoe Tsai <thebrokentoaster@gmail.com>
      b2a3b54b
    • David Crawshaw's avatar
      cmd/go, misc: rework cwd handling for iOS tests · 593ea3b3
      David Crawshaw authored
      Another change in behvaior (bug) in LLDB. Despite the fact that
      LLDB can dump the symtab of our test binaries and show the function
      addresses, it can no longer call the functions. This means the chdir
      trick on signal is failing.
      
      This CL uses a new trick. For iOS, the exec script passes the change
      in directory as an argument, and it is processed early by the test
      harness generated by cmd/go.
      
      For the iOS builders.
      
      Change-Id: I8f5d0f831fe18de99f097761f89c5184d5bf2afb
      Reviewed-on: https://go-review.googlesource.com/35152Reviewed-by: default avatarElias Naur <elias.naur@gmail.com>
      Run-TryBot: David Crawshaw <crawshaw@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      593ea3b3
    • Michael Munday's avatar
      syscall: export Fsid.X__val on s390x · 0642b8a2
      Michael Munday authored
      mkpost.go replaces all variables prefixed with 'X_' with '_' on s390x
      because most of them do not need to be exposed. X__val is being used
      by a third party library so it turns out we do need to expose it on
      s390x (it is already exposed on all other Linux architectures).
      
      Fixes #17298 and updates #18632.
      
      Change-Id: Ic03463229a5f75ca41a4a4b50300da4b4d892d45
      Reviewed-on: https://go-review.googlesource.com/30130Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      0642b8a2
    • Alberto Donizetti's avatar
      doc/gdb: mention GOTRACEBACK=crash · 4601eae6
      Alberto Donizetti authored
      Also fix a couple of other errors.
      
      Fixes #6877
      
      Change-Id: I94c81c5847cc7b0adab19418e71687bc2ee7fe94
      Reviewed-on: https://go-review.googlesource.com/34960Reviewed-by: default avatarIan Lance Taylor <iant@golang.org>
      4601eae6
    • Keith Randall's avatar
      misc/cgo/testplugin: test that types and itabs are unique · 4c4c5fc7
      Keith Randall authored
      Make sure that the same type and itab generated in two
      different plugins are actually the same thing.
      
      See also CL 35115
      
      Change-Id: I0c1ecb039d7e2bf5a601d58dfa162a435ae4ef76
      Reviewed-on: https://go-review.googlesource.com/35116
      Run-TryBot: Keith Randall <khr@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarDavid Crawshaw <crawshaw@golang.org>
      4c4c5fc7
    • Austin Clements's avatar
      reflect: keep makeFuncImpl live across makeFuncStub · 22689c44
      Austin Clements authored
      When traceback sees reflect.makeFuncStub (or reflect.methodValueCall)
      on the stack, it expects to be able to get the *reflect.makeFuncImpl
      (or *reflect.methodValue) for that call from the first outgoing
      argument slot of makeFuncStub/methodValueCall.
      
      However, currently this object isn't necessarily kept live across
      makeFuncStub. This means it may get garbage collected while in a
      reflect call and reused for something else. If we then try to
      traceback, the runtime will see a corrupted makeFuncImpl object and
      panic. This was not a problem in previous releases because we always
      kept arguments live across the whole function. This became a problem
      when we stopped doing this.
      
      Fix this by using reflect.KeepAlive to keep the
      makeFuncImpl/methodValue live across all of callReflect/callMethod,
      which in turn keeps it live as long as makeFuncStub/methodValueCall
      are on the stack.
      
      Fixes #18635.
      
      Change-Id: I91853efcf17912390fddedfb0230648391c33936
      Reviewed-on: https://go-review.googlesource.com/35151
      Run-TryBot: Austin Clements <austin@google.com>
      Reviewed-by: default avatarIan Lance Taylor <iant@golang.org>
      Reviewed-by: default avatarRuss Cox <rsc@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      22689c44
  3. 12 Jan, 2017 7 commits
    • David Crawshaw's avatar
      cmd/link: only exclude C-only symbols on darwin · 9cf06ed6
      David Crawshaw authored
      C-only symbols are excluded from pclntab because of a quirk of darwin,
      where functions are referred to by an exported symbol so dynamic
      relocations de-duplicate to the host binary module and break unwinding.
      
      This doesn't happen on ELF systems because the linker always refers to
      unexported module-local symbols, so we don't need this condition.
      And the current logic for excluding some functions breaks the module
      verification code in moduledataverify1. So disable this for plugins
      on linux.
      
      (In 1.9, it will probably be necessary to introduce a module-local
      symbol reference system on darwin to fix a different bug, so all of
      this onlycsymbol code made be short-lived.)
      
      With this CL, the tests in CL 35116 pass.
      
      Change-Id: I517d7ca4427241fa0a91276c462827efb9383be9
      Reviewed-on: https://go-review.googlesource.com/35190Reviewed-by: default avatarMichael Hudson-Doyle <michael.hudson@canonical.com>
      Run-TryBot: David Crawshaw <crawshaw@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      9cf06ed6
    • Joe Tsai's avatar
      compress/flate: avoid large stack growth in fillDeflate · 9c3630f5
      Joe Tsai authored
      Ranging over an array causes the array to be copied over to the
      stack, which cause large re-growths. Instead, we should iterate
      over slices of the array.
      
      Also, assigning a large struct literal uses the stack even
      though the actual fields being populated are small in comparison
      to the entirety of the struct (see #18636).
      
      Fixing the stack growth does not alter CPU-time performance much
      since the stack-growth and copying was such a tiny portion of the
      compression work:
      
      name                         old time/op    new time/op    delta
      Encode/Digits/Default/1e4-8     332µs ± 1%     332µs ± 1%   ~     (p=0.796 n=10+10)
      Encode/Digits/Default/1e5-8    5.07ms ± 2%    5.05ms ± 1%   ~       (p=0.815 n=9+8)
      Encode/Digits/Default/1e6-8    53.7ms ± 1%    53.9ms ± 1%   ~     (p=0.075 n=10+10)
      Encode/Twain/Default/1e4-8      380µs ± 1%     380µs ± 1%   ~     (p=0.684 n=10+10)
      Encode/Twain/Default/1e5-8     5.79ms ± 2%    5.79ms ± 1%   ~      (p=0.497 n=9+10)
      Encode/Twain/Default/1e6-8     61.5ms ± 1%    61.8ms ± 1%   ~     (p=0.247 n=10+10)
      
      name                         old speed      new speed      delta
      Encode/Digits/Default/1e4-8  30.1MB/s ± 1%  30.1MB/s ± 1%   ~     (p=0.753 n=10+10)
      Encode/Digits/Default/1e5-8  19.7MB/s ± 2%  19.8MB/s ± 1%   ~       (p=0.795 n=9+8)
      Encode/Digits/Default/1e6-8  18.6MB/s ± 1%  18.5MB/s ± 1%   ~     (p=0.072 n=10+10)
      Encode/Twain/Default/1e4-8   26.3MB/s ± 1%  26.3MB/s ± 1%   ~     (p=0.616 n=10+10)
      Encode/Twain/Default/1e5-8   17.3MB/s ± 2%  17.3MB/s ± 1%   ~      (p=0.484 n=9+10)
      Encode/Twain/Default/1e6-8   16.3MB/s ± 1%  16.2MB/s ± 1%   ~     (p=0.238 n=10+10)
      
      Updates #18636
      Fixes #18625
      
      Change-Id: I471b20339bf675f63dc56d38b3acdd824fe23328
      Reviewed-on: https://go-review.googlesource.com/35122Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      Run-TryBot: Joe Tsai <thebrokentoaster@gmail.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      9c3630f5
    • David Crawshaw's avatar
      cmd/go: add comment about SIGUSR2 on iOS · 4f0aac52
      David Crawshaw authored
      Missing from CL 34926.
      
      Change-Id: I4a046440c30811f26da53bee0e853dae3b0ac57a
      Reviewed-on: https://go-review.googlesource.com/35123Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      4f0aac52
    • David Crawshaw's avatar
      cmd/go, misc: switch from breakpoint to SIGUSR2 · 333f764d
      David Crawshaw authored
      The iOS test harness has set a breakpoint early in the life of Go
      programs so that it can change the current working directory using
      information only available from the host debugger. Somewhere in the
      upgrade to iOS 10 / XCode 8.2, breakpoints stopped working. This
      may be an LLDB bug, or a bug in the ios-deploy LLDB scripts, it's
      not clear.
      
      Work around the problem by giving up on breakpoints. Instead, early
      in the life of every test binary built for iOS, send (and ignore) a
      SIGUSR2 signal. The debugger will catch this, giving the script
      go_darwin_arm_exec a chance to change the working directory.
      
      For the iOS builders.
      
      Change-Id: I7476531985217d0c76bc176904c48379210576c2
      Reviewed-on: https://go-review.googlesource.com/34926Reviewed-by: default avatarIan Lance Taylor <iant@golang.org>
      Run-TryBot: Ian Lance Taylor <iant@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      333f764d
    • Shenghou Ma's avatar
      doc/go1.8: update timezone database version · 39e31d5e
      Shenghou Ma authored
      Fixes #18623.
      
      Change-Id: Ic965f5f7088c3270adbca7162226be486d1b9b4e
      Reviewed-on: https://go-review.googlesource.com/35130Reviewed-by: default avatarIan Lance Taylor <iant@golang.org>
      39e31d5e
    • Keith Randall's avatar
      misc/cgo/testshared: test that types and itabs are unique · 08da8201
      Keith Randall authored
      Make sure that the same type and itab generated in two
      different shared library are actually the same thing.
      
      Change-Id: Ica45862d65ff8bc7ad04d59a41f57223f71224cd
      Reviewed-on: https://go-review.googlesource.com/35115
      Run-TryBot: Keith Randall <khr@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarIan Lance Taylor <iant@golang.org>
      08da8201
    • Bryan C. Mills's avatar
      runtime: avoid clobbering C callee-save register in cgoSigtramp · fdde7ba2
      Bryan C. Mills authored
      Use R11 (a caller-saved temp register) instead of RBX (a callee-saved
      register).
      
      I believe this only affects linux/amd64, since it is the only platform
      with a non-trivial cgoSigtramp implementation.
      
      Updates #18328.
      
      Change-Id: I3d35c4512624184d5a8ece653fa09ddf50e079a2
      Reviewed-on: https://go-review.googlesource.com/35068Reviewed-by: default avatarIan Lance Taylor <iant@golang.org>
      Run-TryBot: Ian Lance Taylor <iant@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      fdde7ba2
  4. 11 Jan, 2017 4 commits
  5. 10 Jan, 2017 3 commits
  6. 09 Jan, 2017 3 commits
    • David Chase's avatar
      cmd/compile: insert scheduling checks on loop backedges · 7f1ff65c
      David Chase authored
      Loop breaking with a counter.  Benchmarked (see comments),
      eyeball checked for sanity on popular loops.  This code
      ought to handle loops in general, and properly inserts phi
      functions in cases where the earlier version might not have.
      
      Includes test, plus modifications to test/run.go to deal with
      timeout and killing looping test.  Tests broken by the addition
      of extra code (branch frequency and live vars) for added
      checks turn the check insertion off.
      
      If GOEXPERIMENT=preemptibleloops, the compiler inserts reschedule
      checks on every backedge of every reducible loop.  Alternately,
      specifying GO_GCFLAGS=-d=ssa/insert_resched_checks/on will
      enable it for a single compilation, but because the core Go
      libraries contain some loops that may run long, this is less
      likely to have the desired effect.
      
      This is intended as a tool to help in the study and diagnosis
      of GC and other latency problems, now that goal STW GC latency
      is on the order of 100 microseconds or less.
      
      Updates #17831.
      Updates #10958.
      
      Change-Id: I6206c163a5b0248e3f21eb4fc65f73a179e1f639
      Reviewed-on: https://go-review.googlesource.com/33910
      Run-TryBot: David Chase <drchase@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarKeith Randall <khr@golang.org>
      7f1ff65c
    • Robert Griesemer's avatar
      cmd/compile: file line number for //go:xxx directives · f412bd31
      Robert Griesemer authored
      Minimally invasive; fixes a regression from 1.7.
      
      Fixes #18459.
      
      Change-Id: I93b3b5c05706eaff8ae97a237f770838c1f8778c
      Reviewed-on: https://go-review.googlesource.com/34721Reviewed-by: default avatarDavid Chase <drchase@google.com>
      Reviewed-by: default avatarRuss Cox <rsc@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      f412bd31
    • Joe Tsai's avatar
      net/http: preserve original HTTP method when possible · a8871194
      Joe Tsai authored
      In Go1.7, a 301, 302, or 303 redirect on a HEAD method, would still
      cause the following redirects to still use a HEAD.
      In CL/29852 this behavior was changed such that those codes always
      caused a redirect with the GET method. Fix this such that both
      GET and HEAD will preserve the method.
      
      Fixes #18570
      
      Change-Id: I4bfe69872a30799419e3fad9178f907fe439b449
      Reviewed-on: https://go-review.googlesource.com/34981
      Run-TryBot: Joe Tsai <thebrokentoaster@gmail.com>
      Reviewed-by: default avatarEmmanuel Odeke <emm.odeke@gmail.com>
      Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      a8871194
  7. 08 Jan, 2017 1 commit
  8. 07 Jan, 2017 6 commits
  9. 06 Jan, 2017 7 commits
    • Brad Fitzpatrick's avatar
      doc: update CONTRIBUTING.md a bit, mention proposal process · 5ddfa69f
      Brad Fitzpatrick authored
      Fixes #18550
      
      Change-Id: Ia08d0ef6964216fcc14fa63c2ba378d68daa2c02
      Reviewed-on: https://go-review.googlesource.com/34917Reviewed-by: default avatarChris Broadfoot <cbro@golang.org>
      5ddfa69f
    • Matthew Dempsky's avatar
      net: disable RFC 6724 Rule 9 for IPv4 addresses · 116da1c6
      Matthew Dempsky authored
      Rule 9 arguably doesn't make sense for IPv4 addresses, and so far it
      has only caused problems (#13283, #18518). Disable it until we hear
      from users that actually want/need it.
      
      Fixes #18518.
      
      Change-Id: I7b0dd75d03819cab8e0cd4c29f0c1dc8d2e9c179
      Reviewed-on: https://go-review.googlesource.com/34914Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      Run-TryBot: Matthew Dempsky <mdempsky@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      116da1c6
    • David Chase's avatar
      cmd/compile: rewrite literal.method to ensure full initialization · 41d2278e
      David Chase authored
      CALLPART of STRUCTLIT did not check for incomplete initialization
      of struct; modify PTRLIT treatment to force zeroing.
      
      Test for structlit, believe this might have also failed for
      arraylit.
      
      Fixes #18410.
      
      Change-Id: I511abf8ef850e300996d40568944665714efe1fc
      Reviewed-on: https://go-review.googlesource.com/34622
      Run-TryBot: David Chase <drchase@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarKeith Randall <khr@golang.org>
      41d2278e
    • Jaana Burcu Dogan's avatar
      doc: explain how to set GOPATH to a custom value · a37b9e8e
      Jaana Burcu Dogan authored
      Updates #18294.
      
      Change-Id: Ib6b84243a15ed921cc8960e5fa355fd7594181e6
      Reviewed-on: https://go-review.googlesource.com/34821Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      a37b9e8e
    • Russ Cox's avatar
      runtime: fix corruption crash/race between select and stack growth · b902a63a
      Russ Cox authored
      To implement the blocking of a select, a goroutine builds a list of
      offers to communicate (pseudo-g's, aka sudog), one for each case,
      queues them on the corresponding channels, and waits for another
      goroutine to complete one of those cases and wake it up. Obviously it
      is not OK for two other goroutines to complete multiple cases and both
      wake the goroutine blocked in select. To make sure that only one
      branch of the select is chosen, all the sudogs contain a pointer to a
      shared (single) 'done uint32', which is atomically cas'ed by any
      interested goroutines. The goroutine that wins the cas race gets to
      wake up the select. A complication is that 'done uint32' is stored on
      the stack of the goroutine running the select, and that stack can move
      during the select due to stack growth or stack shrinking.
      
      The relevant ordering to block and unblock in select is:
      
      	1. Lock all channels.
      	2. Create list of sudogs and queue sudogs on all channels.
      	3. Switch to system stack, mark goroutine as asleep,
      	   unlock all channels.
      	4. Sleep until woken.
      	5. Wake up on goroutine stack.
      	6. Lock all channels.
      	7. Dequeue sudogs from all channels.
      	8. Free list of sudogs.
      	9. Unlock all channels.
      
      There are two kinds of stack moves: stack growth and stack shrinking.
      Stack growth happens while the original goroutine is running.
      Stack shrinking happens asynchronously, during garbage collection.
      
      While a channel listing a sudog is locked by select in this process,
      no other goroutine can attempt to complete communication on that
      channel, because that other goroutine doesn't hold the lock and can't
      find the sudog. If the stack moves while all the channel locks are
      held or when the sudogs are not yet or no longer queued in the
      channels, no problem, because no goroutine can get to the sudogs and
      therefore to selectdone. We only need to worry about the stack (and
      'done uint32') moving with the sudogs queued in unlocked channels.
      
      Stack shrinking can happen any time the goroutine is stopped.
      That code already acquires all the channel locks before doing the
      stack move, so it avoids this problem.
      
      Stack growth can happen essentially any time the original goroutine is
      running on its own stack (not the system stack). In the first half of
      the select, all the channels are locked before any sudogs are queued,
      and the channels are not unlocked until the goroutine has stopped
      executing on its own stack and is asleep, so that part is OK. In the
      second half of the select, the goroutine wakes up on its own goroutine
      stack and immediately locks all channels. But the actual call to lock
      might grow the stack, before acquiring any locks. In that case, the
      stack is moving with the sudogs queued in unlocked channels. Not good.
      One goroutine has already won a cas on the old stack (that goroutine
      woke up the selecting goroutine, moving it out of step 4), and the
      fact that done = 1 now should prevent any other goroutines from
      completing any other select cases. During the stack move, however,
      sudog.selectdone is moved from pointing to the old done variable on
      the old stack to a new memory location on the new stack. Another
      goroutine might observe the moved pointer before the new memory
      location has been initialized. If the new memory word happens to be
      zero, that goroutine might win a cas on the new location, thinking it
      can now complete the select (again). It will then complete a second
      communication (reading from or writing to the goroutine stack
      incorrectly) and then attempt to wake up the selecting goroutine,
      which is already awake.
      
      The scribbling over the goroutine stack unexpectedly is already bad,
      but likely to go unnoticed, at least immediately. As for the second
      wakeup, there are a variety of ways it might play out.
      
      * The goroutine might not be asleep.
      That will produce a runtime crash (throw) like in #17007:
      
      	runtime: gp: gp=0xc0422dcb60, goid=2299, gp->atomicstatus=8
      	runtime:  g:  g=0xa5cfe0, goid=0,  g->atomicstatus=0
      	fatal error: bad g->status in ready
      
      Here, atomicstatus=8 is copystack; the second, incorrect wakeup is
      observing that the selecting goroutine is in state "Gcopystack"
      instead of "Gwaiting".
      
      * The goroutine might be sleeping in a send on a nil chan.
      If it wakes up, it will crash with 'fatal error: unreachable'.
      
      * The goroutine might be sleeping in a send on a non-nil chan.
      If it wakes up, it will crash with 'fatal error: chansend:
      spurious wakeup'.
      
      * The goroutine might be sleeping in a receive on a nil chan.
      If it wakes up, it will crash with 'fatal error: unreachable'.
      
      * The goroutine might be sleeping in a receive on a non-nil chan.
      If it wakes up, it will silently (incorrectly!) continue as if it
      received a zero value from a closed channel, leaving a sudog queued on
      the channel pointing at that zero vaue on the goroutine's stack; that
      space will be reused as the goroutine executes, and when some other
      goroutine finally completes the receive, it will do a stray write into
      the goroutine's stack memory, which may cause problems. Then it will
      attempt the real wakeup of the goroutine, leading recursively to any
      of the cases in this list.
      
      * The goroutine might have been running a select in a finalizer
      (I hope not!) and might now be sleeping waiting for more things to
      finalize. If it wakes up, as long as it goes back to sleep quickly
      (before the real GC code tries to wake it), the spurious wakeup does
      no harm (but the stack was still scribbled on).
      
      * The goroutine might be sleeping in gcParkAssist.
      If it wakes up, that will let the goroutine continue executing a bit
      earlier than we would have liked. Eventually the GC will attempt the
      real wakeup of the goroutine, leading recursively to any of the cases
      in this list.
      
      * The goroutine cannot be sleeping in bgsweep, because the background
      sweepers never use select.
      
      * The goroutine might be sleeping in netpollblock.
      If it wakes up, it will crash with 'fatal error: netpollblock:
      corrupted state'.
      
      * The goroutine might be sleeping in main as another thread crashes.
      If it wakes up, it will exit(0) instead of letting the other thread
      crash with a non-zero exit status.
      
      * The goroutine cannot be sleeping in forcegchelper,
      because forcegchelper never uses select.
      
      * The goroutine might be sleeping in an empty select - select {}.
      If it wakes up, it will return to the next line in the program!
      
      * The goroutine might be sleeping in a non-empty select (again).
      In this case, it will wake up spuriously, with gp.param == nil (no
      reason for wakeup), but that was fortuitously overloaded for handling
      wakeup due to a closing channel and the way it is handled is to rerun
      the select, which (accidentally) handles the spurious wakeup
      correctly:
      
      	if cas == nil {
      		// This can happen if we were woken up by a close().
      		// TODO: figure that out explicitly so we don't need this loop.
      		goto loop
      	}
      
      Before looping, it will dequeue all the sudogs on all the channels
      involved, so that no other goroutine will attempt to wake it.
      Since the goroutine was blocked in select before, being blocked in
      select again when the spurious wakeup arrives may be quite likely.
      In this case, the spurious wakeup does no harm (but the stack was
      still scribbled on).
      
      * The goroutine might be sleeping in semacquire (mutex slow path).
      If it wakes up, that is taken as a signal to try for the semaphore
      again, not a signal that the semaphore is now held, but the next
      iteration around the loop will queue the sudog a second time, causing
      a cycle in the wakeup list for the given address. If that sudog is the
      only one in the list, when it is eventually dequeued, it will
      (due to the precise way the code is written) leave the sudog on the
      queue inactive with the sudog broken. But the sudog will also be in
      the free list, and that will eventually cause confusion.
      
      * The goroutine might be sleeping in notifyListWait, for sync.Cond.
      If it wakes up, (*Cond).Wait returns. The docs say "Unlike in other
      systems, Wait cannot return unless awoken by Broadcast or Signal,"
      so the spurious wakeup is incorrect behavior, but most callers do not
      depend on that fact. Eventually the condition will happen, attempting
      the real wakeup of the goroutine and leading recursively to any of the
      cases in this list.
      
      * The goroutine might be sleeping in timeSleep aka time.Sleep.
      If it wakes up, it will continue running, leaving a timer ticking.
      When that time bomb goes off, it will try to ready the goroutine
      again, leading to any one of the cases in this list.
      
      * The goroutine cannot be sleeping in timerproc,
      because timerproc never uses select.
      
      * The goroutine might be sleeping in ReadTrace.
      If it wakes up, it will print 'runtime: spurious wakeup of trace
      reader' and return nil. All future calls to ReadTrace will print
      'runtime: ReadTrace called from multiple goroutines simultaneously'.
      Eventually, when trace data is available, a true wakeup will be
      attempted, leading to any one of the cases in this list.
      
      None of these fatal errors appear in any of the trybot or dashboard
      logs. The 'bad g->status in ready' that happens if the goroutine is
      running (the most likely scenario anyway) has happened once on the
      dashboard and eight times in trybot logs. Of the eight, five were
      atomicstatus=8 during net/http tests, so almost certainly this bug.
      The other three were atomicstatus=2, all near code in select,
      but in a draft CL by Dmitry that was rewriting select and may or may
      not have had its own bugs.
      
      This bug has existed since Go 1.4. Until then the select code was
      implemented in C, 'done uint32' was a C stack variable 'uint32 done',
      and C stacks never moved. I believe it has become more common recently
      because of Brad's work to run more and more tests in net/http in
      parallel, which lengthens race windows.
      
      The fix is to run step 6 on the system stack,
      avoiding possibility of stack growth.
      
      Fixes #17007 and possibly other mysterious failures.
      
      Change-Id: I9d6575a51ac96ae9d67ec24da670426a4a45a317
      Reviewed-on: https://go-review.googlesource.com/34835
      Run-TryBot: Russ Cox <rsc@golang.org>
      Reviewed-by: default avatarAustin Clements <austin@google.com>
      b902a63a
    • Austin Clements's avatar
      runtime: expand HACKING.md · 5dd978a2
      Austin Clements authored
      This adds high-level descriptions of the scheduler structures, the
      user and system stacks, error handling, and synchronization.
      
      Change-Id: I1eed97c6dd4a6e3d351279e967b11c6e64898356
      Reviewed-on: https://go-review.googlesource.com/34290Reviewed-by: default avatarRick Hudson <rlh@golang.org>
      5dd978a2
    • Austin Clements's avatar
      runtime: update big mgc.go comment · 618c2915
      Austin Clements authored
      The comment describing the overall GC algorithm at the top of mgc.go
      has gotten woefully out-of-date (and was possibly never
      correct/complete). Update it to reflect the current workings of the
      GC and the set of phases that we now divide it into.
      
      Change-Id: I02143c0ebefe9d4cd7753349dab8045f0973bf95
      Reviewed-on: https://go-review.googlesource.com/34711Reviewed-by: default avatarRick Hudson <rlh@golang.org>
      618c2915