- 18 Dec, 2015 4 commits
-
-
Austin Clements authored
Currently, if sigprof determines that the G is in user code (not cgo or libcall code), it will only traceback the G stack if it can acquire the stack barrier lock. However, it has no such restriction if the G is in cgo or libcall code. Because cgo calls count as syscalls, stack scanning and stack barrier installation can occur during a cgo call, which means sigprof could attempt to traceback a G in a cgo call while scanstack is installing stack barriers in that G's stack. As a result, the following sequence of events can cause the sigprof traceback to panic with "missed stack barrier": 1. M1: G1 performs a Cgo call (which, on Windows, is any system call, which could explain why this is easier to reproduce on Windows). 2. M1: The Cgo call puts G1 into _Gsyscall state. 3. M2: GC starts a scan of G1's stack. It puts G1 in to _Gscansyscall and acquires the stack barrier lock. 4. M3: A profiling signal comes in. On Windows this is a global (though I don't think this matters), so the runtime stops M1 and calls sigprof for G1. 5. M3: sigprof fails to acquire the stack barrier lock (because the GC's stack scan holds it). 6. M3: sigprof observes that G1 is in a Cgo call, so it calls gentraceback on G1 with its Cgo transition point. 7. M3: gentraceback on G1 grabs the currently empty g.stkbar slice. 8. M2: GC finishes scanning G1's stack and installing stack barriers. 9. M3: gentraceback encounters one of the just-installed stack barriers and panics. This commit fixes this by only allowing cgo tracebacks if sigprof can acquire the stack barrier lock, just like in the regular user traceback case. For good measure, we put the same constraint on libcall tracebacks. This case is probably already safe because, unlike cgo calls, libcalls leave the G in _Grunning and prevent reaching a safe point, so scanstack cannot run during a libcall. However, this also means that sigprof will always acquire the stack barrier lock without contention, so there's no cost to adding this constraint to libcall tracebacks. Fixes #12528. For 1.5.3 (will require some backporting). Change-Id: Ia5a4b8e3d66b23b02ffcd54c6315c81055c0cec2 Reviewed-on: https://go-review.googlesource.com/18023 Run-TryBot: Austin Clements <austin@google.com> Reviewed-by: Russ Cox <rsc@golang.org>
-
Austin Clements authored
Currently, setNextBarrierPC manipulates the stack barriers without acquiring the stack barrier lock. This is mostly okay because setNextBarrierPC also runs synchronously on the G and prevents safe points, but this doesn't prevent a sigprof from occurring during a setNextBarrierPC and performing a traceback. Given that setNextBarrierPC simply sets one entry in the stack barrier array, this is almost certainly safe in reality. However, given that this depends on a subtle argument, which may not hold in the future, and that setNextBarrierPC almost never happens, making it nowhere near performance-critical, we can simply acquire the stack barrier lock and be sure that the synchronization will work. Updates #12528. For 1.5.3. Change-Id: Ife696e10d969f190157eb1cbe762a2de2ebce079 Reviewed-on: https://go-review.googlesource.com/18022 Run-TryBot: Austin Clements <austin@google.com> Reviewed-by: Russ Cox <rsc@golang.org>
-
Emmanuel Odeke authored
Change-Id: I7405cf6f65bccbb07a27f2dc2e3802cab591e296 Reviewed-on: https://go-review.googlesource.com/18030Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
-
Ian Lance Taylor authored
Update #12416. Change-Id: I21d97cbe211ccc8048e5a78ea4d89664f4d195ba Reviewed-on: https://go-review.googlesource.com/17041Reviewed-by: Russ Cox <rsc@golang.org>
-
- 17 Dec, 2015 36 commits
-
-
Shenghou Ma authored
Change-Id: Ie8ec4cfc68abef51e52090a75245f96af874c74a Reviewed-on: https://go-review.googlesource.com/18000Reviewed-by: Ian Lance Taylor <iant@golang.org>
-
Alan Donovan authored
Change-Id: Ic4f4bc7ea7478908716b951815280e394c55310b Reviewed-on: https://go-review.googlesource.com/17975Reviewed-by: Robert Griesemer <gri@golang.org>
-
Brad Fitzpatrick authored
Change-Id: If2b30ab412d6799c8be01eb007462d6b58660ece Reviewed-on: https://go-review.googlesource.com/18014Reviewed-by: Chris Broadfoot <cbro@golang.org> Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org>
-
Brad Fitzpatrick authored
(Found while debugging release problems with go1.6beta1) Updates #12002 Change-Id: Iec197a754205e7fd28be154f27f17f3315886364 Reviewed-on: https://go-review.googlesource.com/18011Reviewed-by: Chris Broadfoot <cbro@golang.org>
-
Brad Fitzpatrick authored
Change-Id: Iae05082530891175e9c86da244e610bc92759561 Reviewed-on: https://go-review.googlesource.com/17918Reviewed-by: Chris Broadfoot <cbro@golang.org>
-
Brad Fitzpatrick authored
Fixes #13660 Change-Id: I05bcb4efcb865192a1ef6756e9dccef83505934c Reviewed-on: https://go-review.googlesource.com/17990Reviewed-by: Chris Broadfoot <cbro@golang.org>
-
Brad Fitzpatrick authored
Update docs on ResponseWriter and Handler around concurrency. Also add a test. The Handler docs were old and used "object" a lot. It was also too ServeMux-centric. Fixes #13050 Updates #13659 (new issue found in http2 while writing the test) Change-Id: I25f53d5fa54f1c9d579d3d0f191bf3d94b1a251b Reviewed-on: https://go-review.googlesource.com/17982Reviewed-by: Russ Cox <rsc@golang.org> Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
-
Russ Cox authored
The test for non-package main top-level inputs is done while parsing the export data. Issue #13468 happened because we were not parsing the export data when using compiler-generated archives (that is, when using go tool compile -pack). Fix this by parsing the export data even for archives. However, that turns up a different problem: the export data check reports (one assumes spurious) skew errors now, because it has not been run since Go 1.2. (Go 1.3 was the first release to use go tool compile -pack.) Since the code hasn't run since Go 1.2, it can't be that important. Since it doesn't work today, just delete it. Figuring out how to make this code work with Robert's export format was one of the largest remaining TODOs for that format. Now we don't have to. Fixes #13468 and makes the world a better place. Change-Id: I40a4b284cf140d49d48b714bd80762d6889acdb9 Reviewed-on: https://go-review.googlesource.com/17976Reviewed-by: Robert Griesemer <gri@golang.org>
-
Russ Cox authored
Since we allow non-200 responses from HTTPS in normal operation, it seems odd to reject them in -insecure operation. Fixes #13037 (again). Change-Id: Ie232f7544ab192addfad407525888db6b967befe Reviewed-on: https://go-review.googlesource.com/17945Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
-
Russ Cox authored
The change here is to move the closeBody call into the if block. The logging adjustments are just arranging to tell the truth: in particular if we're not in insecure mode and we get a non-200 error then we do not actually ignore the response (except as caused by closing the body incorrectly). As the comment below the change indicates, it is intentional that we process non-200 pages. The code does process them, because the if err != nil || status != 200 block does not return. But that block does close the body, which depending on timing can apparently poison the later read from the body. See #13037's initial report: $ go get -v bosun.org/cmd/bosun/cache Fetching https://bosun.org/cmd/bosun/cache?go-get=1 ignoring https fetch with status code 404 Parsing meta tags from https://bosun.org/cmd/bosun/cache?go-get=1 (status code 404) import "bosun.org/cmd/bosun/cache": parsing bosun.org/cmd/bosun/cache: http: read on closed response body package bosun.org/cmd/bosun/cache: unrecognized import path "bosun.org/cmd/bosun/cache" The log print about ignoring the https fetch is not strictly true, since the next thing that happened was parsing the body of that fetch. But the read on the closed response body failed during parsing. Moving the closeBody to happen only when we're about to discard the result and start over (that is, only in -insecure mode) fixes the parse. At least it should fix the parse. I can't seem to break the parse anymore, because of #13648 (close not barring future reads anymore), but this way is clearly better than the old way. If nothing else the old code closed the body twice when err != nil and -insecure was not given. Fixes #13037. Change-Id: Idf57eceb6d5518341a2f7f75eb8f8ab27ed4e0b4 Reviewed-on: https://go-review.googlesource.com/17944Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
-
Russ Cox authored
This caused #13657. Reverting fixes #13657. I was trying to be helpful by fixing #12313, but I don't need the fix myself. Will leave for someone with more motivation. This reverts commit 3e9f0636. Change-Id: Ifc78a6196f23e0f58e3b9ad7340e207a2d5de0a6 Reviewed-on: https://go-review.googlesource.com/17977Reviewed-by: Russ Cox <rsc@golang.org>
-
Brad Fitzpatrick authored
Fixes #11207 Change-Id: I7f00b638e749fbc7907dc1597347ea426367d13e Reviewed-on: https://go-review.googlesource.com/17980 Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Russ Cox <rsc@golang.org>
-
Brad Fitzpatrick authored
Patch from Russ. No bug identified, but I didn't search exhaustively. The new code is easier to read. Fixes #13621 Change-Id: Ifda936e4101116fa254ead950b5fe06adb14e977 Reviewed-on: https://go-review.googlesource.com/17981Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org> Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Russ Cox <rsc@golang.org>
-
Russ Cox authored
Thanks to @toxeus on GitHub for the test case. Fixes #12612. Change-Id: I0c32fbe5044f3552053460a5347c062568093dff Reviewed-on: https://go-review.googlesource.com/17974Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
-
Brad Fitzpatrick authored
Updates to golang.org/x/net/http2 git rev 28273ec9 for https://golang.org/cl/17937 Fixes #13648 Change-Id: I27c77524b2e4a172c5f8be08f6fbb0f2e2e4b200 Reviewed-on: https://go-review.googlesource.com/17938Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org> Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
-
Ian Lance Taylor authored
Fixes #13655. Change-Id: I764019aecdd59743baa436b7339499e6c2126268 Reviewed-on: https://go-review.googlesource.com/17916 Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: Russ Cox <rsc@golang.org>
-
Russ Cox authored
Also update many call sites where I forgot that the permission argument is going to be masked by umask. Fixes #12692. Change-Id: I52b315b06236122ca020950447863fa396b68abd Reviewed-on: https://go-review.googlesource.com/17950Reviewed-by: Ian Lance Taylor <iant@golang.org>
-
Brad Fitzpatrick authored
Should fix nacl. Follow-up to https://golang.org/cl/17936 (fix race) and https://golang.org/cl/17914 (fix build) for https://golang.org/cl/16953 (broke the build) Third time's a charm. Change-Id: I23930d5cff4235209546952ce2231f165ab5bf8a Reviewed-on: https://go-review.googlesource.com/17939 Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
-
Russ Cox authored
Fixes #13577. Change-Id: I0bb8157d6210b0c7c09380c2163b7d7349495732 Reviewed-on: https://go-review.googlesource.com/17970Reviewed-by: Ian Lance Taylor <iant@golang.org>
-
Brad Fitzpatrick authored
This updates the bundled copy of x/net/http2 to git rev d2ecd08 for https://golang.org/cl/17912 (http2: send client trailers) and enables the final Trailer test for http2. Fixes #13557 Change-Id: Iaa15552b82bf7a2cb01b7787a2e1ec5ee680a9d3 Reviewed-on: https://go-review.googlesource.com/17935 Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> Reviewed-by: Russ Cox <rsc@golang.org>
-
Alex Brainman authored
Also include test for interface state (up or down). Updates #13606 Change-Id: I03538d65525ddd9c2d0254761861c2df7fc5bd5a Reviewed-on: https://go-review.googlesource.com/17850Reviewed-by: Russ Cox <rsc@golang.org> Reviewed-by: Mikio Hara <mikioh.mikioh@gmail.com> Run-TryBot: Russ Cox <rsc@golang.org>
-
Brad Fitzpatrick authored
Fixes race builders, broken in https://golang.org/cl/16953 Change-Id: Id61171672b69d0ca412de4b44bf2c598fe557906 Reviewed-on: https://go-review.googlesource.com/17936 Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> Reviewed-by: Russ Cox <rsc@golang.org>
-
Russ Cox authored
This makes go get gitserver/~user/repo.git/foo work. Fixes #9193. Change-Id: I8c9d4096903288f7f0e82d6ed1aa78bf038fb81a Reviewed-on: https://go-review.googlesource.com/17952Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
-
Russ Cox authored
This doesn't happen enough in the tests to be worth debugging. Empirically, I expect this to add 5 seconds to the overall 'go test -short cmd/go' on systems with precise file systems, and nothing on systems without them (like my Mac). Fixes #12205. Change-Id: I0a17cb37bdedcfc0f921c5ee658737f1698c153b Reviewed-on: https://go-review.googlesource.com/17953Reviewed-by: Ian Lance Taylor <iant@golang.org>
-
Russ Cox authored
Fixes #11801. Change-Id: I2caeac7fdddc7f29015d6db8d4b3e296c8b9c423 Reviewed-on: https://go-review.googlesource.com/17954Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
-
Russ Cox authored
Fixes #13639. Fixes #11757. Change-Id: Iecf9ebcd652c23c96477305a41082e5b63b41d83 Reviewed-on: https://go-review.googlesource.com/17955Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
-
Russ Cox authored
No test because the code has no test. Fixes #12313. Change-Id: I2cfd0a0422c0cd76f0371c2d3bbbdf5bb3b3f1eb Reviewed-on: https://go-review.googlesource.com/17951Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
-
Russ Cox authored
Fixes #13538. Change-Id: I621bbe2befe838d16d3664d7a5e30d5d7cceae33 Reviewed-on: https://go-review.googlesource.com/17949Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
-
Russ Cox authored
Fixes #12260. Change-Id: I95c27aad6de8064b9a205d4ee507bce75926f16d Reviewed-on: https://go-review.googlesource.com/17948Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
-
Russ Cox authored
Fixes #12544. Change-Id: I5e2fd1fbb21816e9f6fb022e2664484a71093b04 Reviewed-on: https://go-review.googlesource.com/17947Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
-
Ian Lance Taylor authored
This is an attempt to document the current state of signal handling. It's not intended to describe the best way to handle signals. Future changes to signal handling should update these docs as appropriate. update #9896. Change-Id: I3c50af5cc641357b57dfe90ae1c7883a7e1ec059 Reviewed-on: https://go-review.googlesource.com/17877Reviewed-by: Russ Cox <rsc@golang.org>
-
Russ Cox authored
Fixes #11521. Change-Id: I73615b881df4a0d5e2f5bc5059359d150ca8c105 Reviewed-on: https://go-review.googlesource.com/17946Reviewed-by: Joe Shaw <joe@joeshaw.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
-
Brad Fitzpatrick authored
Fixes #13654 Change-Id: Id2ce32c52efcfdbd66630725d62d2ca6bf0916d5 Reviewed-on: https://go-review.googlesource.com/17934Reviewed-by: Russ Cox <rsc@golang.org>
-
mattn authored
Fixes #6303 Change-Id: Ib2cd15ac6106ef8e6b975943db8efc8d8ab21052 Reviewed-on: https://go-review.googlesource.com/4310Reviewed-by: Russ Cox <rsc@golang.org>
-
Brad Fitzpatrick authored
https://golang.org/cl/16953 broke the world. Change-Id: I7cbd4105338ff896bd0c8f69a0b126b6272be2e5 Reviewed-on: https://go-review.googlesource.com/17914Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
-
Brad Fitzpatrick authored
Document that ListenAndServe and ListenAndServeTLS also set TCP keep-alives. Fixes #12748 Change-Id: Iba2e8a58dd657eba326db49a6c872e2d972883a4 Reviewed-on: https://go-review.googlesource.com/17681Reviewed-by: Russ Cox <rsc@golang.org>
-