Remove some incorrect code that was present after since I added
support for idle timeouts in CL 22670.
This code actually caused a bug (a rare goroutine leak) rather than
prevent a bogus connection reuse.
The t.idleMu mutex already protects most the invariants, including an
explicit Stop call. There's only one Stop call on that timer, and it's
guarded by t.idleMu. What idleMu doesn't protect against is the timer
firing on its own. But we don't need code to protect against that case
because the goroutine that is created via AfterFunc when the timer
fires already checks the invariants:
// closeConnIfStillIdle closes the connection if it's still sitting idle.
// This is what's called by the persistConn's idleTimer, and is run in its
// own goroutine.
func (pc *persistConn) closeConnIfStillIdle() {
t := pc.t
t.idleMu.Lock()
defer t.idleMu.Unlock()
if _, ok := t.idleLRU.m[pc]; !ok {
// Not idle.
return
}
(note the "Not idle." part).
Tested by hand with the repro code from #25621. No more leaks.
Fixes#25621
Change-Id: Idf011a4cb1fcd01f55a5a6269e4c0ee5f4446786
Reviewed-on: https://go-review.googlesource.com/123315
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
OpenBSD 6.4 will require the stack pointer to be pointing at an area that is
marked as MAP_STACK when entering and exiting syscalls. Adjust the stack pointer
used for a new thread such that it points within the stack, not at the top of
it (i.e. outside).
Fixes#26142
Change-Id: I905bd8e5be3dfc325392e7ac490fb56a7c71b3aa
Reviewed-on: https://go-review.googlesource.com/122735
Reviewed-by: Austin Clements <austin@google.com>
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Private fields of the Go class are not used any more after the program
has exited. Delete them to allow JavaScript's garbage collection to
clean up the WebAssembly instance.
Updates #26193.
Change-Id: I349784a49eaad0c22ceedd4f859df97132775537
Reviewed-on: https://go-review.googlesource.com/122296
Run-TryBot: Richard Musiol <neelance@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Paul Jolly <paul@myitcv.org.uk>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
In general, dumb terminal indicates terminal with limited capability.
It may provide no support for special character sequences, e.g., no
handling of ANSI escape sequences. Its input/output handling behavior
may deviate from what's described in termios or terminfo. E.g., in
the shell in emacs, even after successfully setting the terminal to
raw mode, the terminal behaves as if it's still operating in canonical
mode since emacs is doing input processing first.
Readline support can be broken in various ways in dumb terminal mode,
so we want to disable readline or advanced UI features. The easiest
way to detect dumb terminal is to check the environment variable "TERM".
Fixes#26254
Change-Id: I6b652eb555bc03b84405aae08b0b25d111fbb8b0
Reviewed-on: https://go-review.googlesource.com/122879
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Taking a read lock in SetBlocking could cause SetBlocking to block
waiting for a Read in another goroutine to complete. Since SetBlocking
is called by os.(*File).Fd, that could lead to deadlock if the
goroutine calling Fd is going to use it to unblock the Read.
Use an atomic store instead.
Updates #24481
Change-Id: I79413328e06ddf28b6d5b8af7a0e29d5b4e1e6ff
Reviewed-on: https://go-review.googlesource.com/123176
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Recognize NetBSD in:
- go/internal/work/init.go
- race.bash
- runtime/race/race.go
Add __ps_strings symbol in runtime/cgo/netbsd.go as this is
used internally in the TSan library for NetBSD and used for
ReExec().
Tested on NetBSD/amd64 v. 8.99.12.
Around 98% tests are passing for the ./race.bash target.
Updates #19273
Change-Id: Ic0e48d2fb159a7868aab5e17156eeaca1225e513
GitHub-Last-Rev: d6e082707b
GitHub-Pull-Request: golang/go#24322
Reviewed-on: https://go-review.googlesource.com/99835
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
When generating an abstract function DIE, call objabi.PathToPrefix on
the import path so as to be consistent with how the linker handles
import paths. This is intended to resolve another problem with DWARF
inline info generation in which there are multiple inconsistent
versions of an abstract function DIE for a function whose package path
is rewritten/canonicalized by objabi.PathToPrefix.
Fixes#26237
Change-Id: I4b64c090ae43a1ad87f47587a1a71f19bc5fc8e8
Reviewed-on: https://go-review.googlesource.com/123036
Run-TryBot: Than McIntosh <thanm@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Functions exported on behalf of other packages need to have their
argument stack maps specified explicitly. They don't get an implicit
map because they are not in the local package, and if they get defer'd
they need argument maps.
Fixes#24419
Change-Id: I35b7d8b4a03d4770ba88699e1007cb3fcb5397a9
Reviewed-on: https://go-review.googlesource.com/122676
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
At the moment, method declarations are type-checked together with
they receiver base types. This is a known problem (to be fixed early
for Go 1.12) but with the new cycle detection algorithm now also
introduced artifical type cycles.
This change pushes a special marker on the cycle path in those cases
so that these cycles can be ignored.
Fixes#26124.
Change-Id: I64da4ccc32d4ae293da48880c892154a1c6ac3fe
Reviewed-on: https://go-review.googlesource.com/121757
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
The clang compiler on some terminals will issue colored error
messages, which can confuse tools like cgo. To avoid this we used to
set TERM=dumb for all programs started by the go tool. However, that
confuses the pprof tool, which doesn't know whether to support fancy
editing and colors itself.
Instead, change the go tool and the cgo tool to set TERM=dumb where it
matters--when invoking the C compiler--rather than in all cases.
Updates #26254
Change-Id: I95174f961ac269a50a83f5f9d268219043cba968
Reviewed-on: https://go-review.googlesource.com/122975
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
More test cases of combined load for arm64.
Change-Id: I7a9f4dcec6930f161cbded1f47dbf7fcef1db4f1
Reviewed-on: https://go-review.googlesource.com/122582
Run-TryBot: Ben Shi <powerman1st@163.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Also populate Imports for test main with go list -test.
Update comment in internal/load/test.go about
p.Imports, p.Internal.RawImports, and p.Imports
being perfectly aligned. The first two are,
but the third is not, as evidenced by CL 111175.
Since p.Imports is not aligned, don't assume that anymore.
Fixes#25949.
Change-Id: Icbfbc881bc01d1e195a759648fbd1c978ddbc161
Reviewed-on: https://go-review.googlesource.com/122878
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
The existing algorithm assumed that the length of a cycle was simply
the number of elements in the cycle slice starting at the start object.
However, we use a special "indir" indirection object to indicate
pointer and other indirections that break the inline placement of
types in composite types. These indirection objects don't exist as
true named type objects, so don't count them anymore.
This removes an unnecessary cycle error in one of the existing tests
(testdata/issues.src:100).
Also:
- added more tracing support (only active if tracing is enabled)
- better documentation in parts
- r/check.typ/check.typExpr/ in a few of places where we don't
need to record a type indirection
Found while investigating #26124.
Change-Id: I45341743225d979a72af3fbecfa05012b32fab67
Reviewed-on: https://go-review.googlesource.com/121755
Run-TryBot: Robert Griesemer <gri@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
cmd/cover has always assumed that package x/y/z can be
found in $GOPATH/src/x/y/z (roughly; by using go/build).
That won't be true for too much longer. Instead, run the
go command to find out where packages are.
This will make 'go tool cover' safe for use with Go modules
when they are in use in Go 1.11, and it continues to work
with the existing Go toolchains too.
An alternative would be to modify the cover profile format
to record file names directly, but that would require also
updating golang.org/x/tools/cover/profile and any tools
that use it, which seems not worth the trouble.
(That fork of the code does not contain any code to resolve
package names to directory locations, so it's unaffected.)
No new test here: cmd/go's TestCoverageFunc tests this code.
Fixes#25318 (when people use Go 1.11 instead of vgo).
Change-Id: I8769b15107aecf25f7aaf8692b724cf7d0f073d0
Reviewed-on: https://go-review.googlesource.com/122478
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
CL 122518 rolled back an earlier CL that made "go test"
start running test binaries for packages with no *_test.go files.
Add a test as another roadblock to reintroducing that behavior
in the future.
For #26462.
Change-Id: I898103064efee8d6ae65bcf74f4dffc830eae7e9
Reviewed-on: https://go-review.googlesource.com/122595
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
When DWARF is disabled, some alg functions were not generated.
Make sure they are generated when we about to generate calls to
them.
Fixes#23546.
Change-Id: Iecfa0eea830e42ee92e55268167cefb1540980b2
Reviewed-on: https://go-review.googlesource.com/122403
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Adds tests for #122590 and updates x/net/http2 to git rev 6a8eb5e2b1 for:
http2: call httptrace.ClientTrace.GetConn in Transport when needed
https://golang.org/cl/122590
http2: fire httptrace.ClientTrace.WroteHeaderField if set
https://golang.org/cl/122816
http2: compare Connection header value case-insensitively
https://golang.org/cl/122588
This also includes the code for graceful shutdown, but it has no
public API surface via net/http, and should not affect any existing
code paths, as it's purely new stuff:
http2: implement client initiated graceful shutdown
https://golang.org/cl/30076Fixes#19761Fixes#23041
Change-Id: I5558a84591014554cad15ee3852a349ed717530f
Reviewed-on: https://go-review.googlesource.com/122591
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Prior to the fix to #23643, the ReverseProxy didn't panic with
ErrAbortHandler when the copy to a client failed.
During Go 1.11 beta testing, we found plenty of code using
ReverseProxy in tests that were unprepared for a panic.
Change the behavior to only panic when running under the http.Server
that'll handle the panic.
Updates #23643
Change-Id: Ic1fa8405fd54c858ce8c797cec79d006833a9f7d
Reviewed-on: https://go-review.googlesource.com/122819
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The cgo tool records the value of the CGO_LDFLAGS environment variable
in the generated file, so that the linker can later read and use it.
Therefore, we must add CGO_LDFLAGS to the cache ID, as otherwise
changing CGO_LDFLAGS may cause a build result to be incorrectly read
from the cache, producing a different final program.
Change-Id: Ic89c1edc4069837451a36376710ca9b56fb87455
Reviewed-on: https://go-review.googlesource.com/122520
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This commit adds the actual type to the panic message when calling
a method of Value on a Value with a bad type. It also adds better
panic messages to Value.Invoke and Value.Call.
Change-Id: Ic4b3aa29d3bef8e357be40cd07664ad602ffab12
Reviewed-on: https://go-review.googlesource.com/122376
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This commits adds Value.Type(), which returns the JavaScript type of
a Value.
The implementation uses two previously unused bits of the NaN payload
to encode type information.
Change-Id: I568609569983791d50d35b8d80c44f3472203511
Reviewed-on: https://go-review.googlesource.com/122375
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Fix duplicated index in LHS and RHS of the < operator.
Found using https://go-critic.github.io/overview#dupSubExpr-ref
Change-Id: I9a5a40bbd436b32e8117579a01bc50afe3608c97
Reviewed-on: https://go-review.googlesource.com/122776
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
We need to determine whether arguments to and return values from C
functions are "bad" typedef'd pointer types which need to be uintptr
on the Go side.
The type of those arguments are not specified explicitly. As a result,
we never look through the C declarations for the GetTypeID functions
associated with that type, and never realize that they are bad.
However, in another function in the same package there might be an
explicit reference. Then we end up with the declaration being uintptr
in one file and *struct{...} in another file. Badness ensues.
Fix this by doing a 2-pass algorithm. In the first pass, we run as
normal, but record all the argument and result types we see. In the
second pass, we include those argument types also when reading the C
types.
Fixes#24161
Change-Id: I8d727e73a2fbc88cb9d9899f8719ae405f59f753
Reviewed-on: https://go-review.googlesource.com/122575
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Added since last update.
Change-Id: Ic5be0e2e379d422ef72b956a794d65613a0dd7be
Reviewed-on: https://go-review.googlesource.com/122815
Reviewed-by: Andrew Bonventre <andybons@golang.org>
CL 95215 changed text/template so that untyped nil arguments were no
longer ignored, but were instead passed to functions as expected.
This had an unexpected effect on html/template, where all data is
implicitly passed to functions: originally untyped nil arguments were
not passed and were thus effectively ignored, but after CL 95215 they
were passed and were printed, typically as an escaped version of "<nil>".
This CL restores some of the behavior of html/template by ignoring
untyped nil arguments passed implicitly to escaper functions.
While eliminating one change to html/template relative to earlier
releases, this unfortunately introduces a different one: originally
values of interface type with the value nil were printed as an escaped
version of "<nil>". With this CL they are ignored as though they were
untyped nil values. My judgement is that this is a less common case.
We'll see.
This CL adds some tests of typed and untyped nil values to
html/template and text/template to capture the current behavior.
Updates #18716Fixes#25875
Change-Id: I5912983ca32b31ece29e929e72d503b54d7b0cac
Reviewed-on: https://go-review.googlesource.com/121815
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The same-site cookie attribute prevents a cookie from being sent along with
cross-site requests. The main goal is mitigate the risk of cross-origin
information leakage and provides some protection against cross-site request
forgery attacks.
This change adds the option to http.Cookie so it can be stored and
passed to HTTP clients.
Spec: https://tools.ietf.org/html/draft-ietf-httpbis-cookie-same-site-00Fixes#15867
Based on
eb31a0f063
by Reed Loden <reed@hackerone.com>
Change-Id: I98c8a9a92358b2f632990576879759e3aff38cff
Reviewed-on: https://go-review.googlesource.com/79919
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Add field to http.Transport which limits connections per host,
including dial-in-progress, in-use and idle (keep-alive) connections.
For HTTP/2, this field only controls the number of dials in progress.
Fixes#13957
Change-Id: I7a5e045b4d4793c6b5b1a7191e1342cd7df78e6c
Reviewed-on: https://go-review.googlesource.com/71272
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
We need to make sure that the terminating comparison has the right
sense given the increment direction. If the increment is positive,
the terminating comparsion must be < or <=. If the increment is
negative, the terminating comparison must be > or >=.
Do a few cleanups, like constant-folding entry==0, adding comments,
removing unused "exported" fields.
Fixes#26116
Change-Id: I14230ee8126054b750e2a1f2b18eb8f09873dbd5
Reviewed-on: https://go-review.googlesource.com/121940
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Revert CL 101715.
The size of a sync.Pool scales linearly with GOMAXPROCS,
making it inappropriate to put a sync.Pool in any individually
allocated object, as the sync.Pool documentation explains.
The change also broke DeepEqual on regexps.
I have a cleaner way to do this with global sync.Pools but it's
too late in the cycle. Will revisit in Go 1.12. For now, revert.
Fixes#26219.
Change-Id: Ie632e709eb3caf489d85efceac0e4b130ec2019f
Reviewed-on: https://go-review.googlesource.com/122596
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Original CL description:
When using test -cover or -coverprofile the output for "no test files"
is the same format as for "no tests to run".
Reverting because this CL changed cmd/go to build test binaries for
packages that have no tests, leading to extra work and confusion.
Updates #24570Fixes#25789Fixes#26157Fixes#26242
Change-Id: Ibab1307d39dfaec0de9359d6d96706e3910c8efd
Reviewed-on: https://go-review.googlesource.com/122518
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
This test is skipped on Linux and should be skipped on Android for the
same reason.
Change-Id: I753c4788d935bd58874554b455c0d5be2315b794
Reviewed-on: https://go-review.googlesource.com/122585
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
CL 122515 tightened TestAbort to look for breakpoint exceptions and
not just general signal crashes, but this only applies on x86 arches.
On non-x86 arches we use a nil pointer dereference to abort, so the
test is now failing.
This CL re-loosens TestAbort on non-x86 arches to only expect a signal
traceback.
Should fix the build on linux/arm, linux/arm64, linux/ppc64, and
linux/s390x.
Change-Id: I1065341180ab5ab4da63b406c641dcde93c9490b
Reviewed-on: https://go-review.googlesource.com/122580
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Since CL 122515, TestAbort is failing on Plan 9
because there is no SIGTRAP signal on Plan 9,
but a note containing the "sys: breakpoint" string.
This change fixes the TestAbort test by handling
the Plan 9 case.
Fixes#26265.
Change-Id: I2fae00130bcee1cf946d8cc9d147a77f951be390
Reviewed-on: https://go-review.googlesource.com/122464
Run-TryBot: David du Colombier <0intro@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Currently, if the runtime overflows the g0 stack on Windows, it leads
to an infinite recursion:
1. Something overflows the g0 stack bounds and calls morestack.
2. morestack determines it's on the g0 stack and hence cannot grow the
stack, so it calls badmorestackg0 (which prints "fatal: morestack on
g0") followed by abort.
3. abort performs an INT $3, which turns into a Windows
_EXCEPTION_BREAKPOINT exception.
4. This enters the Windows sigtramp, which ensures we're on the g0
stack and calls exceptionhandler.
5. exceptionhandler has a stack check prologue, so it determines that
it's out of stack and calls morestack.
6. goto 2
Fix this by making the exception handler avoid stack checks until it
has ruled out an abort and by blowing away the stack bounds in
lastcontinuehandler before we print the final fatal traceback (which
itself involves a lot of stack bounds checks).
Fixes#21382.
Change-Id: Ie66e91f708e18d131d97f22b43f9ac26f3aece5a
Reviewed-on: https://go-review.googlesource.com/120857
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
Windows includes an 8K guard in system-allocated thread stacks, which
we currently don't account for when setting the g0 stack bounds. As a
result, if we do overflow the g0 stack bounds, we'll get a
STATUS_GUARD_PAGE_VIOLATION exception, which we're not expecting.
Fix the g0 stack bounds to include a total of 16K of slop to account
for this 8K guard.
Updates #21382.
Change-Id: Ia89b741b1413328e4681a237f5a7ee645531fe16
Reviewed-on: https://go-review.googlesource.com/122516
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
