From ed8cbbc3ae96aef98d8f9e9e7003a99ed74992b5 Mon Sep 17 00:00:00 2001 From: Ian Lance Taylor Date: Wed, 21 Jul 2021 19:57:56 -0700 Subject: [release-branch.go1.16] runtime: don't clear timerModifiedEarliest if adjustTimers is 0 This avoids a race when a new timerModifiedEarlier timer is created by a different goroutine. For #47329 Fixes #47332 Change-Id: I6f6c87b4a9b5491b201c725c10bc98e23e0ed9d1 Reviewed-on: https://go-review.googlesource.com/c/go/+/336432 Trust: Ian Lance Taylor Run-TryBot: Ian Lance Taylor TryBot-Result: Go Bot Reviewed-by: Michael Knyszek (cherry picked from commit 798ec73519a7226d6d436e42498a54aed23b8468) Reviewed-on: https://go-review.googlesource.com/c/go/+/336689 --- src/runtime/runtime2.go | 2 +- src/runtime/time.go | 5 ----- src/time/sleep_test.go | 34 ++++++++++++++++++++++++++++++++++ 3 files changed, 35 insertions(+), 6 deletions(-) diff --git a/src/runtime/runtime2.go b/src/runtime/runtime2.go index 9a032d8658..dd375da193 100644 --- a/src/runtime/runtime2.go +++ b/src/runtime/runtime2.go @@ -653,7 +653,7 @@ type p struct { // timerModifiedEarlier status. Because the timer may have been // modified again, there need not be any timer with this value. // This is updated using atomic functions. - // This is 0 if the value is unknown. + // This is 0 if there are no timerModifiedEarlier timers. timerModifiedEarliest uint64 // Per-P GC state diff --git a/src/runtime/time.go b/src/runtime/time.go index dee6a674e4..7b84d2af57 100644 --- a/src/runtime/time.go +++ b/src/runtime/time.go @@ -668,11 +668,6 @@ func adjusttimers(pp *p, now int64) { if verifyTimers { verifyTimerHeap(pp) } - // There are no timers to adjust, so it is safe to clear - // timerModifiedEarliest. Do so in case it is stale. - // Everything will work if we don't do this, - // but clearing here may save future calls to adjusttimers. - atomic.Store64(&pp.timerModifiedEarliest, 0) return } diff --git a/src/time/sleep_test.go b/src/time/sleep_test.go index 6ee0631a85..e0172bf5e0 100644 --- a/src/time/sleep_test.go +++ b/src/time/sleep_test.go @@ -527,6 +527,40 @@ func TestZeroTimer(t *testing.T) { } } +// Test that rapidly moving a timer earlier doesn't cause it to get dropped. +// Issue 47329. +func TestTimerModifiedEarlier(t *testing.T) { + past := Until(Unix(0, 0)) + count := 1000 + fail := 0 + for i := 0; i < count; i++ { + timer := NewTimer(Hour) + for j := 0; j < 10; j++ { + if !timer.Stop() { + <-timer.C + } + timer.Reset(past) + } + + deadline := NewTimer(10 * Second) + defer deadline.Stop() + now := Now() + select { + case <-timer.C: + if since := Since(now); since > 8*Second { + t.Errorf("timer took too long (%v)", since) + fail++ + } + case <-deadline.C: + t.Error("deadline expired") + } + } + + if fail > 0 { + t.Errorf("%d failures", fail) + } +} + // Benchmark timer latency when the thread that creates the timer is busy with // other work and the timers must be serviced by other threads. // https://golang.org/issue/38860 -- cgit v1.2.3-54-g00ecf From ae7943e11b41b5ee98279bf0574942e09f15ce20 Mon Sep 17 00:00:00 2001 From: Ian Lance Taylor Date: Sun, 25 Jul 2021 16:26:13 -0700 Subject: [release-branch.go1.16] runtime: remove adjustTimers counter In CL 336432 we changed adjusttimers so that it no longer cleared timerModifiedEarliest if there were no timersModifiedEarlier timers. This caused some Google internal tests to time out, presumably due to the increased contention on timersLock. We can avoid that by simply not skipping the loop in adjusttimers, which lets us safely clear timerModifiedEarliest. And if we don't skip the loop, then there isn't much reason to keep the count of timerModifiedEarlier timers at all. So remove it. The effect will be that for programs that create some timerModifiedEarlier timers and then remove them all, the program will do an occasional additional loop over all the timers. And, programs that have some timerModifiedEarlier timers will always loop over all the timers, without the quicker exit when they have all been seen. But the loops should not occur all that often, due to timerModifiedEarliest. For #47329 For #47332 Change-Id: I7b244c1244d97b169a3c7fbc8f8a8b115731ddee Reviewed-on: https://go-review.googlesource.com/c/go/+/337309 Trust: Ian Lance Taylor Run-TryBot: Ian Lance Taylor TryBot-Result: Go Bot Reviewed-by: Michael Pratt (cherry picked from commit bfbb288574841f2db2499a580d7bf985a5df4556) Reviewed-on: https://go-review.googlesource.com/c/go/+/338649 --- src/runtime/proc.go | 1 - src/runtime/runtime2.go | 6 ------ src/runtime/time.go | 53 ++++++++----------------------------------------- 3 files changed, 8 insertions(+), 52 deletions(-) diff --git a/src/runtime/proc.go b/src/runtime/proc.go index cf9770587a..3ace2b329b 100644 --- a/src/runtime/proc.go +++ b/src/runtime/proc.go @@ -4751,7 +4751,6 @@ func (pp *p) destroy() { moveTimers(plocal, pp.timers) pp.timers = nil pp.numTimers = 0 - pp.adjustTimers = 0 pp.deletedTimers = 0 atomic.Store64(&pp.timer0When, 0) unlock(&pp.timersLock) diff --git a/src/runtime/runtime2.go b/src/runtime/runtime2.go index dd375da193..e9825327d1 100644 --- a/src/runtime/runtime2.go +++ b/src/runtime/runtime2.go @@ -699,12 +699,6 @@ type p struct { // Modified using atomic instructions. numTimers uint32 - // Number of timerModifiedEarlier timers on P's heap. - // This should only be modified while holding timersLock, - // or while the timer status is in a transient state - // such as timerModifying. - adjustTimers uint32 - // Number of timerDeleted timers in P's heap. // Modified using atomic instructions. deletedTimers uint32 diff --git a/src/runtime/time.go b/src/runtime/time.go index 7b84d2af57..666b242316 100644 --- a/src/runtime/time.go +++ b/src/runtime/time.go @@ -333,7 +333,6 @@ func deltimer(t *timer) bool { // Must fetch t.pp before setting status // to timerDeleted. tpp := t.pp.ptr() - atomic.Xadd(&tpp.adjustTimers, -1) if !atomic.Cas(&t.status, timerModifying, timerDeleted) { badTimer() } @@ -510,20 +509,9 @@ loop: tpp := t.pp.ptr() - // Update the adjustTimers field. Subtract one if we - // are removing a timerModifiedEarlier, add one if we - // are adding a timerModifiedEarlier. - adjust := int32(0) - if status == timerModifiedEarlier { - adjust-- - } if newStatus == timerModifiedEarlier { - adjust++ updateTimerModifiedEarliest(tpp, when) } - if adjust != 0 { - atomic.Xadd(&tpp.adjustTimers, adjust) - } // Set the new status of the timer. if !atomic.Cas(&t.status, timerModifying, newStatus) { @@ -591,9 +579,6 @@ func cleantimers(pp *p) { // Move t to the right position. dodeltimer0(pp) doaddtimer(pp, t) - if s == timerModifiedEarlier { - atomic.Xadd(&pp.adjustTimers, -1) - } if !atomic.Cas(&t.status, timerMoving, timerWaiting) { badTimer() } @@ -664,32 +649,23 @@ func moveTimers(pp *p, timers []*timer) { // it also moves timers that have been modified to run later, // and removes deleted timers. The caller must have locked the timers for pp. func adjusttimers(pp *p, now int64) { - if atomic.Load(&pp.adjustTimers) == 0 { - if verifyTimers { - verifyTimerHeap(pp) - } - return - } - // If we haven't yet reached the time of the first timerModifiedEarlier // timer, don't do anything. This speeds up programs that adjust // a lot of timers back and forth if the timers rarely expire. // We'll postpone looking through all the adjusted timers until // one would actually expire. - if first := atomic.Load64(&pp.timerModifiedEarliest); first != 0 { - if int64(first) > now { - if verifyTimers { - verifyTimerHeap(pp) - } - return + first := atomic.Load64(&pp.timerModifiedEarliest) + if first == 0 || int64(first) > now { + if verifyTimers { + verifyTimerHeap(pp) } - - // We are going to clear all timerModifiedEarlier timers. - atomic.Store64(&pp.timerModifiedEarliest, 0) + return } + // We are going to clear all timerModifiedEarlier timers. + atomic.Store64(&pp.timerModifiedEarliest, 0) + var moved []*timer -loop: for i := 0; i < len(pp.timers); i++ { t := pp.timers[i] if t.pp.ptr() != pp { @@ -716,11 +692,6 @@ loop: // loop to skip some other timer. dodeltimer(pp, i) moved = append(moved, t) - if s == timerModifiedEarlier { - if n := atomic.Xadd(&pp.adjustTimers, -1); int32(n) <= 0 { - break loop - } - } // Look at this heap position again. i-- } @@ -819,9 +790,6 @@ func runtimer(pp *p, now int64) int64 { t.when = t.nextwhen dodeltimer0(pp) doaddtimer(pp, t) - if s == timerModifiedEarlier { - atomic.Xadd(&pp.adjustTimers, -1) - } if !atomic.Cas(&t.status, timerMoving, timerWaiting) { badTimer() } @@ -916,7 +884,6 @@ func clearDeletedTimers(pp *p) { atomic.Store64(&pp.timerModifiedEarliest, 0) cdel := int32(0) - cearlier := int32(0) to := 0 changedHeap := false timers := pp.timers @@ -941,9 +908,6 @@ nextTimer: if !atomic.Cas(&t.status, timerMoving, timerWaiting) { badTimer() } - if s == timerModifiedEarlier { - cearlier++ - } continue nextTimer } case timerDeleted: @@ -980,7 +944,6 @@ nextTimer: atomic.Xadd(&pp.deletedTimers, -cdel) atomic.Xadd(&pp.numTimers, -cdel) - atomic.Xadd(&pp.adjustTimers, -cearlier) timers = timers[:to] pp.timers = timers -- cgit v1.2.3-54-g00ecf From accf363d5da864521c90b152fb734f3f15e00521 Mon Sep 17 00:00:00 2001 From: Damien Neil Date: Wed, 7 Jul 2021 16:34:34 -0700 Subject: [release-branch.go1.16] net/http/httputil: close incoming ReverseProxy request body Reading from an incoming request body after the request handler aborts with a panic can cause a panic, becuse http.Server does not (contrary to its documentation) close the request body in this case. Always close the incoming request body in ReverseProxy.ServeHTTP to ensure that any in-flight outgoing requests using the body do not read from it. Fixes #47474 Updates #46866 Fixes CVE-2021-36221 Change-Id: I310df269200ad8732c5d9f1a2b00de68725831df Reviewed-on: https://go-review.googlesource.com/c/go/+/333191 Trust: Damien Neil Reviewed-by: Brad Fitzpatrick Reviewed-by: Filippo Valsorda (cherry picked from commit b7a85e0003cedb1b48a1fd3ae5b746ec6330102e) Reviewed-on: https://go-review.googlesource.com/c/go/+/338551 Trust: Filippo Valsorda Run-TryBot: Filippo Valsorda TryBot-Result: Go Bot Reviewed-by: Damien Neil --- src/net/http/httputil/reverseproxy.go | 9 +++++++ src/net/http/httputil/reverseproxy_test.go | 39 ++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+) diff --git a/src/net/http/httputil/reverseproxy.go b/src/net/http/httputil/reverseproxy.go index ccb84562e0..17e6a1dc7a 100644 --- a/src/net/http/httputil/reverseproxy.go +++ b/src/net/http/httputil/reverseproxy.go @@ -234,6 +234,15 @@ func (p *ReverseProxy) ServeHTTP(rw http.ResponseWriter, req *http.Request) { if req.ContentLength == 0 { outreq.Body = nil // Issue 16036: nil Body for http.Transport retries } + if outreq.Body != nil { + // Reading from the request body after returning from a handler is not + // allowed, and the RoundTrip goroutine that reads the Body can outlive + // this handler. This can lead to a crash if the handler panics (see + // Issue 46866). Although calling Close doesn't guarantee there isn't + // any Read in flight after the handle returns, in practice it's safe to + // read after closing it. + defer outreq.Body.Close() + } if outreq.Header == nil { outreq.Header = make(http.Header) // Issue 33142: historical behavior was to always allocate } diff --git a/src/net/http/httputil/reverseproxy_test.go b/src/net/http/httputil/reverseproxy_test.go index 3211463bcb..46da4022c7 100644 --- a/src/net/http/httputil/reverseproxy_test.go +++ b/src/net/http/httputil/reverseproxy_test.go @@ -1121,6 +1121,45 @@ func TestReverseProxy_PanicBodyError(t *testing.T) { rproxy.ServeHTTP(httptest.NewRecorder(), req) } +// Issue #46866: panic without closing incoming request body causes a panic +func TestReverseProxy_PanicClosesIncomingBody(t *testing.T) { + backend := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + out := "this call was relayed by the reverse proxy" + // Coerce a wrong content length to induce io.ErrUnexpectedEOF + w.Header().Set("Content-Length", fmt.Sprintf("%d", len(out)*2)) + fmt.Fprintln(w, out) + })) + defer backend.Close() + backendURL, err := url.Parse(backend.URL) + if err != nil { + t.Fatal(err) + } + proxyHandler := NewSingleHostReverseProxy(backendURL) + proxyHandler.ErrorLog = log.New(io.Discard, "", 0) // quiet for tests + frontend := httptest.NewServer(proxyHandler) + defer frontend.Close() + frontendClient := frontend.Client() + + var wg sync.WaitGroup + for i := 0; i < 2; i++ { + wg.Add(1) + go func() { + defer wg.Done() + for j := 0; j < 10; j++ { + const reqLen = 6 * 1024 * 1024 + req, _ := http.NewRequest("POST", frontend.URL, &io.LimitedReader{R: neverEnding('x'), N: reqLen}) + req.ContentLength = reqLen + resp, _ := frontendClient.Transport.RoundTrip(req) + if resp != nil { + io.Copy(io.Discard, resp.Body) + resp.Body.Close() + } + } + }() + } + wg.Wait() +} + func TestSelectFlushInterval(t *testing.T) { tests := []struct { name string -- cgit v1.2.3-54-g00ecf From 0fb1e1438b02072bb9c3c566b40ad66b1ffed365 Mon Sep 17 00:00:00 2001 From: Roland Shoemaker Date: Fri, 12 Mar 2021 09:51:50 -0800 Subject: [release-branch.go1.16] cmd/go/internal/load: always set IsImportCycle when in a cycle When hitting an import cycle in reusePackage, and there is already an error set, make sure IsImportCycle is set so that we don't end up stuck in a loop. Updates #25830 Fixes #47348 Change-Id: Iba966aea4a637dfc34ee22782a477209ac48c9bd Reviewed-on: https://go-review.googlesource.com/c/go/+/301289 Trust: Roland Shoemaker Run-TryBot: Roland Shoemaker Reviewed-by: Bryan C. Mills Reviewed-by: Jay Conrod TryBot-Result: Go Bot (cherry picked from commit cdd08e615a9b92742b21a94443720b6d70452510) Reviewed-on: https://go-review.googlesource.com/c/go/+/336649 Trust: Bryan C. Mills Run-TryBot: Bryan C. Mills --- src/cmd/go/internal/load/pkg.go | 5 +++++ src/cmd/go/testdata/script/list_err_cycle.txt | 15 +++++++++++++++ 2 files changed, 20 insertions(+) create mode 100644 src/cmd/go/testdata/script/list_err_cycle.txt diff --git a/src/cmd/go/internal/load/pkg.go b/src/cmd/go/internal/load/pkg.go index 8b12faf4cd..61fde895f8 100644 --- a/src/cmd/go/internal/load/pkg.go +++ b/src/cmd/go/internal/load/pkg.go @@ -1323,6 +1323,11 @@ func reusePackage(p *Package, stk *ImportStack) *Package { Err: errors.New("import cycle not allowed"), IsImportCycle: true, } + } else if !p.Error.IsImportCycle { + // If the error is already set, but it does not indicate that + // we are in an import cycle, set IsImportCycle so that we don't + // end up stuck in a loop down the road. + p.Error.IsImportCycle = true } p.Incomplete = true } diff --git a/src/cmd/go/testdata/script/list_err_cycle.txt b/src/cmd/go/testdata/script/list_err_cycle.txt new file mode 100644 index 0000000000..44b82a62b0 --- /dev/null +++ b/src/cmd/go/testdata/script/list_err_cycle.txt @@ -0,0 +1,15 @@ +# Check that we don't get infinite recursion when loading a package with +# an import cycle and another error. Verifies #25830. +! go list +stderr 'found packages a \(a.go\) and b \(b.go\)' + +-- go.mod -- +module errcycle + +go 1.16 +-- a.go -- +package a + +import _ "errcycle" +-- b.go -- +package b \ No newline at end of file -- cgit v1.2.3-54-g00ecf From 3d5afa9610c5b0e45783868ae964152855a736ac Mon Sep 17 00:00:00 2001 From: Than McIntosh Date: Thu, 15 Jul 2021 16:29:36 -0400 Subject: [release-branch.go1.16] cmd/{compile,link}: fix bug in map.zero handling In CL 326211 a change was made to switch "go.map.zero" symbols from non-pkg DUPOK symbols to hashed symbols. The intent of this change was ensure that in cases where there are multiple competing go.map.zero symbols feeding into a link, the largest map.zero symbol is selected. The change was buggy, however, and resulted in duplicate symbols in the final binary (see bug cited below for details). This duplication was relatively benign for linux/ELF, but causes duplicate definition errors on Windows. This patch switches "go.map.zero" symbols back from hashed symbols to non-pkg DUPOK symbols, and updates the relevant code in the loader to ensure that we do the right thing when there are multiple competing DUPOK symbols with different sizes. Fixes #47289. Change-Id: I8aeb910c65827f5380144d07646006ba553c9251 Reviewed-on: https://go-review.googlesource.com/c/go/+/334930 Trust: Than McIntosh Run-TryBot: Than McIntosh TryBot-Result: Go Bot Reviewed-by: Cherry Mui (cherry picked from commit 49402bee36fd3d5cee9f4b2d2e1e8560ead0203b) Reviewed-on: https://go-review.googlesource.com/c/go/+/335629 --- src/cmd/compile/internal/gc/obj.go | 2 +- src/cmd/link/internal/loader/loader.go | 9 ++++ test/fixedbugs/issue47185.dir/bad/bad.go | 72 ++++++++++++++++++++++++++++++++ test/fixedbugs/issue47185.dir/main.go | 28 +++++++++++++ test/fixedbugs/issue47185.go | 11 +++++ 5 files changed, 121 insertions(+), 1 deletion(-) create mode 100644 test/fixedbugs/issue47185.dir/bad/bad.go create mode 100644 test/fixedbugs/issue47185.dir/main.go create mode 100644 test/fixedbugs/issue47185.go diff --git a/src/cmd/compile/internal/gc/obj.go b/src/cmd/compile/internal/gc/obj.go index 579e8bd823..da1869e945 100644 --- a/src/cmd/compile/internal/gc/obj.go +++ b/src/cmd/compile/internal/gc/obj.go @@ -163,7 +163,7 @@ func dumpdata() { if zerosize > 0 { zero := mappkg.Lookup("zero") ggloblsym(zero.Linksym(), int32(zerosize), obj.DUPOK|obj.RODATA) - zero.Linksym().Set(obj.AttrContentAddressable, true) + zero.Linksym().Set(obj.AttrStatic, true) } addGCLocals() diff --git a/src/cmd/link/internal/loader/loader.go b/src/cmd/link/internal/loader/loader.go index 971cc432ff..85b948990a 100644 --- a/src/cmd/link/internal/loader/loader.go +++ b/src/cmd/link/internal/loader/loader.go @@ -474,6 +474,15 @@ func (st *loadState) addSym(name string, ver int, r *oReader, li uint32, kind in if l.flags&FlagStrictDups != 0 { l.checkdup(name, r, li, oldi) } + // Fix for issue #47185 -- given two dupok symbols with + // different sizes, favor symbol with larger size. See + // also issue #46653. + szdup := l.SymSize(oldi) + sz := int64(r.Sym(li).Siz()) + if szdup < sz { + // new symbol overwrites old symbol. + l.objSyms[oldi] = objSym{r.objidx, li} + } return oldi } oldr, oldli := l.toLocal(oldi) diff --git a/test/fixedbugs/issue47185.dir/bad/bad.go b/test/fixedbugs/issue47185.dir/bad/bad.go new file mode 100644 index 0000000000..1aa4fbb909 --- /dev/null +++ b/test/fixedbugs/issue47185.dir/bad/bad.go @@ -0,0 +1,72 @@ +// Copyright 2021 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package a + +// Note that the use of CGO here is solely to trigger external +// linking, since that is required to trigger that bad behavior +// in this bug. + +// #include +import "C" + +func Bad() { + m := make(map[int64]A) + a := m[0] + if len(a.B.C1.D2.E2.F1) != 0 || + len(a.B.C1.D2.E2.F2) != 0 || + len(a.B.C1.D2.E2.F3) != 0 || + len(a.B.C1.D2.E2.F4) != 0 || + len(a.B.C1.D2.E2.F5) != 0 || + len(a.B.C1.D2.E2.F6) != 0 || + len(a.B.C1.D2.E2.F7) != 0 || + len(a.B.C1.D2.E2.F8) != 0 || + len(a.B.C1.D2.E2.F9) != 0 || + len(a.B.C1.D2.E2.F10) != 0 || + len(a.B.C1.D2.E2.F11) != 0 || + len(a.B.C1.D2.E2.F16) != 0 { + panic("bad") + } + C.malloc(100) +} + +type A struct { + B +} + +type B struct { + C1 C + C2 C +} + +type C struct { + D1 D + D2 D +} + +type D struct { + E1 E + E2 E + E3 E + E4 E +} + +type E struct { + F1 string + F2 string + F3 string + F4 string + F5 string + F6 string + F7 string + F8 string + F9 string + F10 string + F11 string + F12 string + F13 string + F14 string + F15 string + F16 string +} diff --git a/test/fixedbugs/issue47185.dir/main.go b/test/fixedbugs/issue47185.dir/main.go new file mode 100644 index 0000000000..7b46e55d8b --- /dev/null +++ b/test/fixedbugs/issue47185.dir/main.go @@ -0,0 +1,28 @@ +// Copyright 2021 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package main + +import ( + bad "issue47185.dir/bad" +) + +func main() { + another() + bad.Bad() +} + +func another() L { + m := make(map[string]L) + return m[""] +} + +type L struct { + A Data + B Data +} + +type Data struct { + F1 [22][]string +} diff --git a/test/fixedbugs/issue47185.go b/test/fixedbugs/issue47185.go new file mode 100644 index 0000000000..9c921b8698 --- /dev/null +++ b/test/fixedbugs/issue47185.go @@ -0,0 +1,11 @@ +// +build cgo +// runindir + +// Copyright 2021 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +// Another test to verify compiler and linker handling of multiple +// competing map.zero symbol definitions. + +package ignored -- cgit v1.2.3-54-g00ecf From 8b6ae9be125dd7c66123686fe339203f0661d92c Mon Sep 17 00:00:00 2001 From: Cherry Zhang Date: Fri, 28 Jun 2019 09:30:36 -0400 Subject: [release-branch.go1.16] cmd/compile: mark R16, R17 clobbered for non-standard calls on ARM64 On ARM64, (external) linker generated trampoline may clobber R16 and R17. In CL 183842 we change Duff's devices not to use those registers. However, this is not enough. The register allocator also needs to know that these registers may be clobbered in any calls that don't follow the standard Go calling convention. This include Duff's devices and the write barrier. Fixes #46928. Updates #32773. Change-Id: Ia52a891d9bbb8515c927617dd53aee5af5bd9aa4 Reviewed-on: https://go-review.googlesource.com/c/go/+/184437 Run-TryBot: Cherry Zhang TryBot-Result: Go Bot Reviewed-by: Meng Zhuo Reviewed-by: Keith Randall Trust: Meng Zhuo (cherry picked from commit 11b4aee05bfe83513cf08f83091e5aef8b33e766) Reviewed-on: https://go-review.googlesource.com/c/go/+/331029 Trust: Cherry Mui Run-TryBot: Cherry Mui --- src/cmd/compile/internal/ssa/gen/ARM64Ops.go | 9 ++++-- src/cmd/compile/internal/ssa/opGen.go | 6 ++-- src/runtime/asm_arm64.s | 47 +++++++++++++--------------- 3 files changed, 31 insertions(+), 31 deletions(-) diff --git a/src/cmd/compile/internal/ssa/gen/ARM64Ops.go b/src/cmd/compile/internal/ssa/gen/ARM64Ops.go index b0bc9c78ff..4d1d14e18b 100644 --- a/src/cmd/compile/internal/ssa/gen/ARM64Ops.go +++ b/src/cmd/compile/internal/ssa/gen/ARM64Ops.go @@ -502,13 +502,14 @@ func init() { // auxint = offset into duffzero code to start executing // returns mem // R20 changed as side effect + // R16 and R17 may be clobbered by linker trampoline. { name: "DUFFZERO", aux: "Int64", argLength: 2, reg: regInfo{ inputs: []regMask{buildReg("R20")}, - clobbers: buildReg("R20 R30"), + clobbers: buildReg("R16 R17 R20 R30"), }, faultOnNilArg0: true, unsafePoint: true, // FP maintenance around DUFFZERO can be clobbered by interrupts @@ -542,13 +543,14 @@ func init() { // auxint = offset into duffcopy code to start executing // returns mem // R20, R21 changed as side effect + // R16 and R17 may be clobbered by linker trampoline. { name: "DUFFCOPY", aux: "Int64", argLength: 3, reg: regInfo{ inputs: []regMask{buildReg("R21"), buildReg("R20")}, - clobbers: buildReg("R20 R21 R26 R30"), + clobbers: buildReg("R16 R17 R20 R21 R26 R30"), }, faultOnNilArg0: true, faultOnNilArg1: true, @@ -707,7 +709,8 @@ func init() { // LoweredWB invokes runtime.gcWriteBarrier. arg0=destptr, arg1=srcptr, arg2=mem, aux=runtime.gcWriteBarrier // It saves all GP registers if necessary, // but clobbers R30 (LR) because it's a call. - {name: "LoweredWB", argLength: 3, reg: regInfo{inputs: []regMask{buildReg("R2"), buildReg("R3")}, clobbers: (callerSave &^ gpg) | buildReg("R30")}, clobberFlags: true, aux: "Sym", symEffect: "None"}, + // R16 and R17 may be clobbered by linker trampoline. + {name: "LoweredWB", argLength: 3, reg: regInfo{inputs: []regMask{buildReg("R2"), buildReg("R3")}, clobbers: (callerSave &^ gpg) | buildReg("R16 R17 R30")}, clobberFlags: true, aux: "Sym", symEffect: "None"}, // There are three of these functions so that they can have three different register inputs. // When we check 0 <= c <= cap (A), then 0 <= b <= c (B), then 0 <= a <= b (C), we want the diff --git a/src/cmd/compile/internal/ssa/opGen.go b/src/cmd/compile/internal/ssa/opGen.go index e590f6ba5d..a3f9a22389 100644 --- a/src/cmd/compile/internal/ssa/opGen.go +++ b/src/cmd/compile/internal/ssa/opGen.go @@ -20848,7 +20848,7 @@ var opcodeTable = [...]opInfo{ inputs: []inputInfo{ {0, 1048576}, // R20 }, - clobbers: 537919488, // R20 R30 + clobbers: 538116096, // R16 R17 R20 R30 }, }, { @@ -20876,7 +20876,7 @@ var opcodeTable = [...]opInfo{ {0, 2097152}, // R21 {1, 1048576}, // R20 }, - clobbers: 607125504, // R20 R21 R26 R30 + clobbers: 607322112, // R16 R17 R20 R21 R26 R30 }, }, { @@ -21373,7 +21373,7 @@ var opcodeTable = [...]opInfo{ {0, 4}, // R2 {1, 8}, // R3 }, - clobbers: 9223372035244163072, // R30 F0 F1 F2 F3 F4 F5 F6 F7 F8 F9 F10 F11 F12 F13 F14 F15 F16 F17 F18 F19 F20 F21 F22 F23 F24 F25 F26 F27 F28 F29 F30 F31 + clobbers: 9223372035244359680, // R16 R17 R30 F0 F1 F2 F3 F4 F5 F6 F7 F8 F9 F10 F11 F12 F13 F14 F15 F16 F17 F18 F19 F20 F21 F22 F23 F24 F25 F26 F27 F28 F29 F30 F31 }, }, { diff --git a/src/runtime/asm_arm64.s b/src/runtime/asm_arm64.s index a09172f0c9..a2eb8bbeb7 100644 --- a/src/runtime/asm_arm64.s +++ b/src/runtime/asm_arm64.s @@ -1155,10 +1155,10 @@ TEXT ·checkASM(SB),NOSPLIT,$0-1 // It does not clobber any general-purpose registers, // but may clobber others (e.g., floating point registers) // The act of CALLing gcWriteBarrier will clobber R30 (LR). -TEXT runtime·gcWriteBarrier(SB),NOSPLIT,$216 +TEXT runtime·gcWriteBarrier(SB),NOSPLIT,$200 // Save the registers clobbered by the fast path. - MOVD R0, 200(RSP) - MOVD R1, 208(RSP) + MOVD R0, 184(RSP) + MOVD R1, 192(RSP) MOVD g_m(g), R0 MOVD m_p(R0), R0 MOVD (p_wbBuf+wbBuf_next)(R0), R1 @@ -1174,8 +1174,8 @@ TEXT runtime·gcWriteBarrier(SB),NOSPLIT,$216 // Is the buffer full? (flags set in CMP above) BEQ flush ret: - MOVD 200(RSP), R0 - MOVD 208(RSP), R1 + MOVD 184(RSP), R0 + MOVD 192(RSP), R1 // Do the write. MOVD R3, (R2) RET @@ -1199,17 +1199,16 @@ flush: MOVD R13, 96(RSP) MOVD R14, 104(RSP) MOVD R15, 112(RSP) - MOVD R16, 120(RSP) - MOVD R17, 128(RSP) + // R16, R17 may be clobbered by linker trampoline // R18 is unused. - MOVD R19, 136(RSP) - MOVD R20, 144(RSP) - MOVD R21, 152(RSP) - MOVD R22, 160(RSP) - MOVD R23, 168(RSP) - MOVD R24, 176(RSP) - MOVD R25, 184(RSP) - MOVD R26, 192(RSP) + MOVD R19, 120(RSP) + MOVD R20, 128(RSP) + MOVD R21, 136(RSP) + MOVD R22, 144(RSP) + MOVD R23, 152(RSP) + MOVD R24, 160(RSP) + MOVD R25, 168(RSP) + MOVD R26, 176(RSP) // R27 is temp register. // R28 is g. // R29 is frame pointer (unused). @@ -1233,16 +1232,14 @@ flush: MOVD 96(RSP), R13 MOVD 104(RSP), R14 MOVD 112(RSP), R15 - MOVD 120(RSP), R16 - MOVD 128(RSP), R17 - MOVD 136(RSP), R19 - MOVD 144(RSP), R20 - MOVD 152(RSP), R21 - MOVD 160(RSP), R22 - MOVD 168(RSP), R23 - MOVD 176(RSP), R24 - MOVD 184(RSP), R25 - MOVD 192(RSP), R26 + MOVD 120(RSP), R19 + MOVD 128(RSP), R20 + MOVD 136(RSP), R21 + MOVD 144(RSP), R22 + MOVD 152(RSP), R23 + MOVD 160(RSP), R24 + MOVD 168(RSP), R25 + MOVD 176(RSP), R26 JMP ret // Note: these functions use a special calling convention to save generated code space. -- cgit v1.2.3-54-g00ecf From 4c62fd36776c03547f7a9f8dc67c8dac8e987851 Mon Sep 17 00:00:00 2001 From: Constantin Konstantinidis Date: Sat, 5 Jun 2021 07:48:30 +0200 Subject: [release-branch.go1.16] cmd/go: remove hint when no module is suggested Updates #46528 Fixes #46551 Change-Id: I2453d321ece878ea7823865758aa4a16b3ed7fe8 Reviewed-on: https://go-review.googlesource.com/c/go/+/325430 Reviewed-by: Bryan C. Mills Run-TryBot: Bryan C. Mills Trust: Heschi Kreinick Trust: Dmitri Shuralyov TryBot-Result: Go Bot (cherry picked from commit e552a6d31270c86064632af1d092e0db5a930250) Reviewed-on: https://go-review.googlesource.com/c/go/+/334371 Trust: Bryan C. Mills Reviewed-by: Jay Conrod Reviewed-by: Michael Matloob --- src/cmd/go/internal/modload/import.go | 10 ++++++---- src/cmd/go/testdata/script/mod_install_hint.txt | 5 +++++ 2 files changed, 11 insertions(+), 4 deletions(-) create mode 100644 src/cmd/go/testdata/script/mod_install_hint.txt diff --git a/src/cmd/go/internal/modload/import.go b/src/cmd/go/internal/modload/import.go index 995641c9f1..eda0568f07 100644 --- a/src/cmd/go/internal/modload/import.go +++ b/src/cmd/go/internal/modload/import.go @@ -160,11 +160,13 @@ func (e *ImportMissingSumError) Error() string { // Importing package is unknown, or the missing package was named on the // command line. Recommend 'go mod download' for the modules that could // provide the package, since that shouldn't change go.mod. - args := make([]string, len(e.mods)) - for i, mod := range e.mods { - args[i] = mod.Path + if len(e.mods) > 0 { + args := make([]string, len(e.mods)) + for i, mod := range e.mods { + args[i] = mod.Path + } + hint = fmt.Sprintf("; to add:\n\tgo mod download %s", strings.Join(args, " ")) } - hint = fmt.Sprintf("; to add:\n\tgo mod download %s", strings.Join(args, " ")) } else { // Importing package is known (common case). Recommend 'go get' on the // current version of the importing package. diff --git a/src/cmd/go/testdata/script/mod_install_hint.txt b/src/cmd/go/testdata/script/mod_install_hint.txt new file mode 100644 index 0000000000..ab02840eb8 --- /dev/null +++ b/src/cmd/go/testdata/script/mod_install_hint.txt @@ -0,0 +1,5 @@ +# Module is replaced but not required. No hint appears as no module is suggested. +go mod init m +go mod edit -replace=github.com/notrequired@v0.5.0=github.com/doesnotexist@v0.5.0 +! go install github.com/notrequired +! stderr 'to add it:' \ No newline at end of file -- cgit v1.2.3-54-g00ecf From 37c117f2bf227b14b0be3e1c2411174a6f27b4fe Mon Sep 17 00:00:00 2001 From: Yasuhiro Matsumoto Date: Wed, 23 Jun 2021 01:02:33 +0900 Subject: [release-branch.go1.16] cmd/go: use path.Dir instead of filepath.Dir for package paths in 'go mod vendor' copyMetadata walk-up to parent directory until the pkg become modPath. But pkg should be slash-separated paths. It have to use path.Dir instead of filepath.Dir. Updates #46867 Fixes #47015 Change-Id: I44cf1429fe52379a7415b94cc30ae3275cc430e8 Reviewed-on: https://go-review.googlesource.com/c/go/+/330149 Reviewed-by: Bryan C. Mills Trust: Bryan C. Mills Trust: Alexander Rakoczy Trust: Carlos Amedee Run-TryBot: Bryan C. Mills TryBot-Result: Go Bot (cherry picked from commit 835d86a17ebf32a3cb081f66119c74363dbd8825) Reviewed-on: https://go-review.googlesource.com/c/go/+/332329 Reviewed-by: Jay Conrod --- src/cmd/go/internal/modcmd/vendor.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/cmd/go/internal/modcmd/vendor.go b/src/cmd/go/internal/modcmd/vendor.go index ac1fb7720a..4ef8481b43 100644 --- a/src/cmd/go/internal/modcmd/vendor.go +++ b/src/cmd/go/internal/modcmd/vendor.go @@ -13,6 +13,7 @@ import ( "io" "io/fs" "os" + "path" "path/filepath" "sort" "strings" @@ -281,7 +282,7 @@ func copyMetadata(modPath, pkg, dst, src string, copiedFiles map[string]bool) { if modPath == pkg { break } - pkg = filepath.Dir(pkg) + pkg = path.Dir(pkg) dst = filepath.Dir(dst) src = filepath.Dir(src) } -- cgit v1.2.3-54-g00ecf From 16ab7e49d4070c4f68e88836b123dbe6da8bb015 Mon Sep 17 00:00:00 2001 From: Derek Parker Date: Thu, 17 Jun 2021 20:22:40 +0000 Subject: [release-branch.go1.16] runtime: fix crash during VDSO calls on PowerPC This patch reinstates a fix for PowerPC with regard to making VDSO calls while receiving a signal, and subsequently crashing. The crash happens because certain VDSO calls can modify the r30 register, which is where g is stored. This change was reverted for PowerPC because r30 is supposed to be a non-volatile register. This is true, but that only makes a guarantee across function calls, but not "within" a function call. This patch was seemingly fine before because the Linux kernel still had hand rolled assembly VDSO function calls, however with a recent change to C function calls it seems the compiler used can generate instructions which temporarily clobber r30. This means that when we receive a signal during one of these calls the value of r30 will not be the g as the runtime expects, causing a segfault. You can see from this assembly dump how the register is clobbered during the call: (the following is from a 5.13rc2 kernel) ``` Dump of assembler code for function __cvdso_clock_gettime_data: 0x00007ffff7ff0700 <+0>: cmplwi r4,15 0x00007ffff7ff0704 <+4>: bgt 0x7ffff7ff07f0 <__cvdso_clock_gettime_data+240> 0x00007ffff7ff0708 <+8>: li r9,1 0x00007ffff7ff070c <+12>: slw r9,r9,r4 0x00007ffff7ff0710 <+16>: andi. r10,r9,2179 0x00007ffff7ff0714 <+20>: beq 0x7ffff7ff0810 <__cvdso_clock_gettime_data+272> 0x00007ffff7ff0718 <+24>: rldicr r10,r4,4,59 0x00007ffff7ff071c <+28>: lis r9,32767 0x00007ffff7ff0720 <+32>: std r30,-16(r1) 0x00007ffff7ff0724 <+36>: std r31,-8(r1) 0x00007ffff7ff0728 <+40>: add r6,r3,r10 0x00007ffff7ff072c <+44>: ori r4,r9,65535 0x00007ffff7ff0730 <+48>: lwz r8,0(r3) 0x00007ffff7ff0734 <+52>: andi. r9,r8,1 0x00007ffff7ff0738 <+56>: bne 0x7ffff7ff07d0 <__cvdso_clock_gettime_data+208> 0x00007ffff7ff073c <+60>: lwsync 0x00007ffff7ff0740 <+64>: mftb r30 <---- RIGHT HERE => 0x00007ffff7ff0744 <+68>: ld r12,40(r6) ``` What I believe is happening is that the kernel changed the PowerPC VDSO calls to use standard C calls instead of using hand rolled assembly. The hand rolled assembly calls never touched r30, so this change was safe to roll back. That does not seem to be the case anymore as on the 5.13rc2 kernel the compiler *is* generating assembly which modifies r30, making this change again unsafe and causing a crash when the program receives a signal during these calls (which will happen often due to async preempt). This change happened here: https://lwn.net/ml/linux-kernel/235e5571959cfa89ced081d7e838ed5ff38447d2.1601365870.git.christophe.leroy@csgroup.eu/. I realize this was reverted due to unexplained hangs in PowerPC builders, but I think we should reinstate this change and investigate those issues separately: https://github.com/golang/go/commit/f4ca3c1e0a2066ca4f7bd6203866d282ed34acf2 Fixes #46858 Change-Id: Ib18d7bbfc80a1a9cb558f0098878d41081324b52 GitHub-Last-Rev: c3002bcfca3ef58b27485e31328e6297b7a9dfe7 GitHub-Pull-Request: golang/go#46767 Reviewed-on: https://go-review.googlesource.com/c/go/+/328110 Run-TryBot: Lynn Boger TryBot-Result: Go Bot Reviewed-by: Cherry Mui Trust: Lynn Boger (cherry picked from commit 16e82be454cbf41299e6a055d54d489ca4612ee0) Reviewed-on: https://go-review.googlesource.com/c/go/+/334410 Run-TryBot: Cherry Mui --- src/runtime/signal_unix.go | 2 +- src/runtime/sys_linux_ppc64x.s | 86 +++++++++++++++++++++++++++++++++++------- 2 files changed, 74 insertions(+), 14 deletions(-) diff --git a/src/runtime/signal_unix.go b/src/runtime/signal_unix.go index 3f70707ab4..89f936ea88 100644 --- a/src/runtime/signal_unix.go +++ b/src/runtime/signal_unix.go @@ -381,7 +381,7 @@ func preemptM(mp *m) { //go:nosplit func sigFetchG(c *sigctxt) *g { switch GOARCH { - case "arm", "arm64": + case "arm", "arm64", "ppc64", "ppc64le": if !iscgo && inVDSOPage(c.sigpc()) { // When using cgo, we save the g on TLS and load it from there // in sigtramp. Just use that. diff --git a/src/runtime/sys_linux_ppc64x.s b/src/runtime/sys_linux_ppc64x.s index fd69ee70a5..7be8c4c724 100644 --- a/src/runtime/sys_linux_ppc64x.s +++ b/src/runtime/sys_linux_ppc64x.s @@ -215,15 +215,45 @@ TEXT runtime·walltime1(SB),NOSPLIT,$16-12 MOVD (g_sched+gobuf_sp)(R7), R1 // Set SP to g0 stack noswitch: - SUB $16, R1 // Space for results - RLDICR $0, R1, $59, R1 // Align for C code + SUB $16, R1 // Space for results + RLDICR $0, R1, $59, R1 // Align for C code MOVD R12, CTR MOVD R1, R4 - BL (CTR) // Call from VDSO - MOVD $0, R0 // Restore R0 - MOVD 0(R1), R3 // sec - MOVD 8(R1), R5 // nsec - MOVD R15, R1 // Restore SP + + // Store g on gsignal's stack, so if we receive a signal + // during VDSO code we can find the g. + // If we don't have a signal stack, we won't receive signal, + // so don't bother saving g. + // When using cgo, we already saved g on TLS, also don't save + // g here. + // Also don't save g if we are already on the signal stack. + // We won't get a nested signal. + MOVBZ runtime·iscgo(SB), R22 + CMP R22, $0 + BNE nosaveg + MOVD m_gsignal(R21), R22 // g.m.gsignal + CMP R22, $0 + BEQ nosaveg + + CMP g, R22 + BEQ nosaveg + MOVD (g_stack+stack_lo)(R22), R22 // g.m.gsignal.stack.lo + MOVD g, (R22) + + BL (CTR) // Call from VDSO + + MOVD $0, (R22) // clear g slot, R22 is unchanged by C code + + JMP finish + +nosaveg: + BL (CTR) // Call from VDSO + +finish: + MOVD $0, R0 // Restore R0 + MOVD 0(R1), R3 // sec + MOVD 8(R1), R5 // nsec + MOVD R15, R1 // Restore SP // Restore vdsoPC, vdsoSP // We don't worry about being signaled between the two stores. @@ -235,7 +265,7 @@ noswitch: MOVD 32(R1), R6 MOVD R6, m_vdsoPC(R21) -finish: +return: MOVD R3, sec+0(FP) MOVW R5, nsec+8(FP) RET @@ -246,7 +276,7 @@ fallback: SYSCALL $SYS_clock_gettime MOVD 32(R1), R3 MOVD 40(R1), R5 - JMP finish + JMP return TEXT runtime·nanotime1(SB),NOSPLIT,$16-8 MOVD $1, R3 // CLOCK_MONOTONIC @@ -282,7 +312,37 @@ noswitch: RLDICR $0, R1, $59, R1 // Align for C code MOVD R12, CTR MOVD R1, R4 - BL (CTR) // Call from VDSO + + // Store g on gsignal's stack, so if we receive a signal + // during VDSO code we can find the g. + // If we don't have a signal stack, we won't receive signal, + // so don't bother saving g. + // When using cgo, we already saved g on TLS, also don't save + // g here. + // Also don't save g if we are already on the signal stack. + // We won't get a nested signal. + MOVBZ runtime·iscgo(SB), R22 + CMP R22, $0 + BNE nosaveg + MOVD m_gsignal(R21), R22 // g.m.gsignal + CMP R22, $0 + BEQ nosaveg + + CMP g, R22 + BEQ nosaveg + MOVD (g_stack+stack_lo)(R22), R22 // g.m.gsignal.stack.lo + MOVD g, (R22) + + BL (CTR) // Call from VDSO + + MOVD $0, (R22) // clear g slot, R22 is unchanged by C code + + JMP finish + +nosaveg: + BL (CTR) // Call from VDSO + +finish: MOVD $0, R0 // Restore R0 MOVD 0(R1), R3 // sec MOVD 8(R1), R5 // nsec @@ -298,7 +358,7 @@ noswitch: MOVD 32(R1), R6 MOVD R6, m_vdsoPC(R21) -finish: +return: // sec is in R3, nsec in R5 // return nsec in R3 MOVD $1000000000, R4 @@ -313,7 +373,7 @@ fallback: SYSCALL $SYS_clock_gettime MOVD 32(R1), R3 MOVD 40(R1), R5 - JMP finish + JMP return TEXT runtime·rtsigprocmask(SB),NOSPLIT|NOFRAME,$0-28 MOVW how+0(FP), R3 @@ -366,7 +426,7 @@ TEXT sigtramp<>(SB),NOSPLIT,$64 // this might be called in external code context, // where g is not set. MOVBZ runtime·iscgo(SB), R6 - CMP R6, $0 + CMP R6, $0 BEQ 2(PC) BL runtime·load_g(SB) -- cgit v1.2.3-54-g00ecf From a6ca6d90b3ca1f0a9fab6cde41079176c1d2094c Mon Sep 17 00:00:00 2001 From: Damien Neil Date: Tue, 3 Aug 2021 19:38:37 -0700 Subject: [release-branch.go1.16] net/http: speed up and deflake TestCancelRequestWhenSharingConnection This test made many requests over the same connection for 10 seconds, trusting that this will exercise the request cancelation race from #41600. Change the test to exhibit the specific race in a targeted fashion with only two requests. Fixes #47535. Updates #41600. Updates #47016. Change-Id: If99c9b9331ff645f6bb67fe9fb79b8aab8784710 Reviewed-on: https://go-review.googlesource.com/c/go/+/339594 Trust: Damien Neil Run-TryBot: Damien Neil TryBot-Result: Go Bot Reviewed-by: Heschi Kreinick (cherry picked from commit 6e738868a7a943d7d4fd6bb1963e7f6d78111726) Reviewed-on: https://go-review.googlesource.com/c/go/+/339830 --- src/net/http/transport_test.go | 77 ++++++++++++++++++++++++++++-------------- 1 file changed, 51 insertions(+), 26 deletions(-) diff --git a/src/net/http/transport_test.go b/src/net/http/transport_test.go index 73d259dc9a..9a665ae7b6 100644 --- a/src/net/http/transport_test.go +++ b/src/net/http/transport_test.go @@ -6433,10 +6433,11 @@ func TestErrorWriteLoopRace(t *testing.T) { // Test that a new request which uses the connection of an active request // cannot cause it to be canceled as well. func TestCancelRequestWhenSharingConnection(t *testing.T) { - if testing.Short() { - t.Skip("skipping in short mode") - } + reqc := make(chan chan struct{}, 2) ts := httptest.NewServer(HandlerFunc(func(w ResponseWriter, req *Request) { + ch := make(chan struct{}, 1) + reqc <- ch + <-ch w.Header().Add("Content-Length", "0") })) defer ts.Close() @@ -6448,34 +6449,58 @@ func TestCancelRequestWhenSharingConnection(t *testing.T) { var wg sync.WaitGroup - ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + wg.Add(1) + putidlec := make(chan chan struct{}) + go func() { + defer wg.Done() + ctx := httptrace.WithClientTrace(context.Background(), &httptrace.ClientTrace{ + PutIdleConn: func(error) { + // Signal that the idle conn has been returned to the pool, + // and wait for the order to proceed. + ch := make(chan struct{}) + putidlec <- ch + <-ch + }, + }) + req, _ := NewRequestWithContext(ctx, "GET", ts.URL, nil) + res, err := client.Do(req) + if err == nil { + res.Body.Close() + } + if err != nil { + t.Errorf("request 1: got err %v, want nil", err) + } + }() - for i := 0; i < 10; i++ { - wg.Add(1) - go func() { - defer wg.Done() - for ctx.Err() == nil { - reqctx, reqcancel := context.WithCancel(ctx) - go reqcancel() - req, _ := NewRequestWithContext(reqctx, "GET", ts.URL, nil) - res, err := client.Do(req) - if err == nil { - res.Body.Close() - } - } - }() - } + // Wait for the first request to receive a response and return the + // connection to the idle pool. + r1c := <-reqc + close(r1c) + idlec := <-putidlec - for ctx.Err() == nil { - req, _ := NewRequest("GET", ts.URL, nil) - if res, err := client.Do(req); err != nil { - t.Errorf("unexpected: %p %v", req, err) - break - } else { + wg.Add(1) + cancelctx, cancel := context.WithCancel(context.Background()) + go func() { + defer wg.Done() + req, _ := NewRequestWithContext(cancelctx, "GET", ts.URL, nil) + res, err := client.Do(req) + if err == nil { res.Body.Close() } - } + if !errors.Is(err, context.Canceled) { + t.Errorf("request 2: got err %v, want Canceled", err) + } + }() + // Wait for the second request to arrive at the server, and then cancel + // the request context. + r2c := <-reqc cancel() + + // Give the cancelation a moment to take effect, and then unblock the first request. + time.Sleep(1 * time.Millisecond) + close(idlec) + + close(r2c) wg.Wait() } -- cgit v1.2.3-54-g00ecf From fa6aa872225f8d33a90d936e7a81b64d2cea68e1 Mon Sep 17 00:00:00 2001 From: David Chase Date: Wed, 4 Aug 2021 19:11:35 +0000 Subject: [release-branch.go1.16] go1.16.7 Change-Id: I5a8616596c53b43f60487e19385b6a60af1addfe Reviewed-on: https://go-review.googlesource.com/c/go/+/339451 Run-TryBot: David Chase TryBot-Result: Go Bot Reviewed-by: Damien Neil Reviewed-by: Alexander Rakoczy Trust: Damien Neil Trust: Carlos Amedee --- VERSION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/VERSION b/VERSION index 3ef20fc6e6..3ddf36dcfe 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -go1.16.6 \ No newline at end of file +go1.16.7 \ No newline at end of file -- cgit v1.2.3-54-g00ecf