From b51123001f76d67df07fca304b16664344ba6800 Mon Sep 17 00:00:00 2001
From: Ian Lance Taylor
+The GCC 4.9 releases include a complete Go 1.2 implementation. +
+
From 5361b747b732cf6cf183591b7a66b83fb6fab29f Mon Sep 17 00:00:00 2001
From: Robert Griesemer unsafe, known to the compiler,
provides facilities for low-level programming including operations
that violate the type system. A package using unsafe
-must be vetted manually for type safety. The package provides the
-following interface:
+must be vetted manually for type safety and may not be portable.
+The package provides the following interface:
@@ -6223,10 +6223,11 @@ func Sizeof(variable ArbitraryType) uintptr
-Any pointer or value of underlying type uintptr can be converted to
-a Pointer type and vice versa.
A Pointer is a pointer type but a Pointer
value may not be dereferenced.
+Any pointer or value of underlying type uintptr can be converted to
+a Pointer type and vice versa.
+The effect of converting between Pointer and uintptr is implementation-defined.
From 6ad2749dcd614270ac58c5254b6ada3bce0af090 Mon Sep 17 00:00:00 2001 From: Russ CoxDate: Thu, 23 Oct 2014 23:44:47 -0400 Subject: [PATCH 03/14] encoding/csv: for Postgres, unquote empty strings, quote \. In theory both of these lines encode the same three fields: a,,c a,"",c However, Postgres defines that when importing CSV, the unquoted version is treated as NULL (missing), while the quoted version is treated as a string value (empty string). If the middle field is supposed to be an integer value, the first line can be imported (NULL is okay), but the second line cannot (empty string is not). Postgres's import command (COPY FROM) has an option to force the unquoted empty to be interpreted as a string but it does not have an option to force the quoted empty to be interpreted as a NULL. From http://www.postgresql.org/docs/9.0/static/sql-copy.html: The CSV format has no standard way to distinguish a NULL value from an empty string. PostgreSQL's COPY handles this by quoting. A NULL is output as the NULL parameter string and is not quoted, while a non-NULL value matching the NULL parameter string is quoted. For example, with the default settings, a NULL is written as an unquoted empty string, while an empty string data value is written with double quotes (""). Reading values follows similar rules. You can use FORCE_NOT_NULL to prevent NULL input comparisons for specific columns. Therefore printing the unquoted empty is more flexible for imports into Postgres than printing the quoted empty. In addition to making the output more useful with Postgres, not quoting empty strings makes the output smaller and easier to read. It also matches the behavior of Microsoft Excel and Google Drive. Since we are here and making concessions for Postgres, handle this case too (again quoting the Postgres docs): Because backslash is not a special character in the CSV format, \., the end-of-data marker, could also appear as a data value. To avoid any misinterpretation, a \. data value appearing as a lone entry on a line is automatically quoted on output, and on input, if quoted, is not interpreted as the end-of-data marker. If you are loading a file created by another application that has a single unquoted column and might have a value of \., you might need to quote that value in the input file. Fixes #7586. LGTM=bradfitz R=bradfitz CC=golang-codereviews https://golang.org/cl/164760043 --- src/encoding/csv/writer.go | 16 ++++++++++++++-- src/encoding/csv/writer_test.go | 11 +++++++++++ 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/src/encoding/csv/writer.go b/src/encoding/csv/writer.go index 1faecb6648..17e7bb7f5c 100644 --- a/src/encoding/csv/writer.go +++ b/src/encoding/csv/writer.go @@ -115,10 +115,22 @@ func (w *Writer) WriteAll(records [][]string) (err error) { } // fieldNeedsQuotes returns true if our field must be enclosed in quotes. -// Empty fields, files with a Comma, fields with a quote or newline, and +// Fields with a Comma, fields with a quote or newline, and // fields which start with a space must be enclosed in quotes. +// We used to quote empty strings, but we do not anymore (as of Go 1.4). +// The two representations should be equivalent, but Postgres distinguishes +// quoted vs non-quoted empty string during database imports, and it has +// an option to force the quoted behavior for non-quoted CSV but it has +// no option to force the non-quoted behavior for quoted CSV, making +// CSV with quoted empty strings strictly less useful. +// Not quoting the empty string also makes this package match the behavior +// of Microsoft Excel and Google Drive. +// For Postgres, quote the data termating string `\.`. func (w *Writer) fieldNeedsQuotes(field string) bool { - if len(field) == 0 || strings.IndexRune(field, w.Comma) >= 0 || strings.IndexAny(field, "\"\r\n") >= 0 { + if field == "" { + return false + } + if field == `\.` || strings.IndexRune(field, w.Comma) >= 0 || strings.IndexAny(field, "\"\r\n") >= 0 { return true } diff --git a/src/encoding/csv/writer_test.go b/src/encoding/csv/writer_test.go index 22b740c074..8ddca0abe0 100644 --- a/src/encoding/csv/writer_test.go +++ b/src/encoding/csv/writer_test.go @@ -28,6 +28,17 @@ var writeTests = []struct { {Input: [][]string{{"abc\ndef"}}, Output: "\"abc\r\ndef\"\r\n", UseCRLF: true}, {Input: [][]string{{"abc\rdef"}}, Output: "\"abcdef\"\r\n", UseCRLF: true}, {Input: [][]string{{"abc\rdef"}}, Output: "\"abc\rdef\"\n", UseCRLF: false}, + {Input: [][]string{{""}}, Output: "\n"}, + {Input: [][]string{{"", ""}}, Output: ",\n"}, + {Input: [][]string{{"", "", ""}}, Output: ",,\n"}, + {Input: [][]string{{"", "", "a"}}, Output: ",,a\n"}, + {Input: [][]string{{"", "a", ""}}, Output: ",a,\n"}, + {Input: [][]string{{"", "a", "a"}}, Output: ",a,a\n"}, + {Input: [][]string{{"a", "", ""}}, Output: "a,,\n"}, + {Input: [][]string{{"a", "", "a"}}, Output: "a,,a\n"}, + {Input: [][]string{{"a", "a", ""}}, Output: "a,a,\n"}, + {Input: [][]string{{"a", "a", "a"}}, Output: "a,a,a\n"}, + {Input: [][]string{{`\.`}}, Output: "\"\\.\"\n"}, } func TestWrite(t *testing.T) { From 737a9e0da87da05f8655c5a2ab258a679cc3f641 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Fri, 24 Oct 2014 00:48:34 -0400 Subject: [PATCH 04/14] doc/go1.4: encoding/csv CC=golang-codereviews https://golang.org/cl/162140043 --- doc/go1.4.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/doc/go1.4.txt b/doc/go1.4.txt index ae52562df9..78f46a3296 100644 --- a/doc/go1.4.txt +++ b/doc/go1.4.txt @@ -26,6 +26,7 @@ crypto/tls: add support for ALPN (RFC 7301) (CL 108710046) crypto/tls: support programmatic selection of server certificates (CL 107400043) flag: it is now an error to set a flag multiple times (CL 156390043) fmt: print type *map[T]T as &map[k:v] (CL 154870043) +encoding/csv: do not quote empty strings, quote \. (CL 164760043) encoding/gob: remove unsafe (CL 102680045) misc: deleted editor support; refer to https://code.google.com/p/go-wiki/wiki/IDEsAndTextEditorPlugins instead (CL 105470043) net/http: add Request.BasicAuth method (CL 76540043) From 5225854b74b11cb374b7398132ec9f1d7abf9820 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Fri, 24 Oct 2014 10:27:39 -0400 Subject: [PATCH 05/14] cmd/gc: synthesize zeroed value for non-assignment context CL 157910047 introduced code to turn a node representing a zeroed composite literal into N, the nil Node* pointer (which represents any zero, not the Go literal nil). That's great for assignments like x = T{}, but it doesn't work when T{} is used in a value context like T{}.v or x == T{}. Fix those. Should have no effect on performance; confirmed. The deltas below are noise (compare ns/op): benchmark old ns/op new ns/op delta BenchmarkBinaryTree17 2902919192 2915228424 +0.42% BenchmarkFannkuch11 2597417605 2630363685 +1.27% BenchmarkFmtFprintfEmpty 73.7 74.8 +1.49% BenchmarkFmtFprintfString 196 199 +1.53% BenchmarkFmtFprintfInt 213 217 +1.88% BenchmarkFmtFprintfIntInt 336 356 +5.95% BenchmarkFmtFprintfPrefixedInt 289 294 +1.73% BenchmarkFmtFprintfFloat 415 416 +0.24% BenchmarkFmtManyArgs 1281 1271 -0.78% BenchmarkGobDecode 10271734 10307978 +0.35% BenchmarkGobEncode 8985021 9079442 +1.05% BenchmarkGzip 410233227 412266944 +0.50% BenchmarkGunzip 102114554 103272443 +1.13% BenchmarkHTTPClientServer 45297 44993 -0.67% BenchmarkJSONEncode 19499741 19498489 -0.01% BenchmarkJSONDecode 76436733 74247497 -2.86% BenchmarkMandelbrot200 4273814 4307292 +0.78% BenchmarkGoParse 4024594 4028937 +0.11% BenchmarkRegexpMatchEasy0_32 131 135 +3.05% BenchmarkRegexpMatchEasy0_1K 328 333 +1.52% BenchmarkRegexpMatchEasy1_32 115 117 +1.74% BenchmarkRegexpMatchEasy1_1K 931 948 +1.83% BenchmarkRegexpMatchMedium_32 216 217 +0.46% BenchmarkRegexpMatchMedium_1K 72669 72857 +0.26% BenchmarkRegexpMatchHard_32 3818 3809 -0.24% BenchmarkRegexpMatchHard_1K 121398 121945 +0.45% BenchmarkRevcomp 613996550 615145436 +0.19% BenchmarkTemplate 93678525 93267391 -0.44% BenchmarkTimeParse 414 411 -0.72% BenchmarkTimeFormat 396 399 +0.76% Fixes #8947. LGTM=r R=r, dave CC=golang-codereviews https://golang.org/cl/162130043 --- src/cmd/gc/walk.c | 8 +----- test/fixedbugs/issue8947.go | 53 +++++++++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 7 deletions(-) create mode 100644 test/fixedbugs/issue8947.go diff --git a/src/cmd/gc/walk.c b/src/cmd/gc/walk.c index 7649728d37..b761662d14 100644 --- a/src/cmd/gc/walk.c +++ b/src/cmd/gc/walk.c @@ -614,7 +614,7 @@ walkexpr(Node **np, NodeList **init) if(oaslit(n, init)) goto ret; - if(n->right == N) + if(n->right == N || iszero(n->right) && !flag_race) goto ret; switch(n->right->op) { @@ -1390,12 +1390,6 @@ walkexpr(Node **np, NodeList **init) case OMAPLIT: case OSTRUCTLIT: case OPTRLIT: - // NOTE(rsc): Race detector cannot handle seeing - // a STRUCTLIT or ARRAYLIT representing a zero value, - // so make a temporary for those always in race mode. - // Otherwise, leave zero values in place. - if(iszero(n) && !flag_race) - goto ret; var = temp(n->type); anylit(0, n, var, init); n = var; diff --git a/test/fixedbugs/issue8947.go b/test/fixedbugs/issue8947.go new file mode 100644 index 0000000000..f40c02e998 --- /dev/null +++ b/test/fixedbugs/issue8947.go @@ -0,0 +1,53 @@ +// run + +// Copyright 2014 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. + +// Some uses of zeroed constants in non-assignment +// expressions broke with our more aggressive zeroing +// of assignments (internal compiler errors). + +package main + +func f1() { + type T [2]int + p := T{0, 1} + switch p { + case T{0, 0}: + panic("wrong1") + case T{0, 1}: + // ok + default: + panic("wrong2") + } + + if p == (T{0, 0}) { + panic("wrong3") + } else if p == (T{0, 1}) { + // ok + } else { + panic("wrong4") + } +} + +type T struct { + V int +} + +var X = T{}.V + +func f2() { + var x = T{}.V + if x != 0 { + panic("wrongx") + } + if X != 0 { + panic("wrongX") + } +} + +func main() { + f1() + f2() +} From c5943c668b919b98fd107c2327678ee32a868246 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Fri, 24 Oct 2014 10:58:13 -0400 Subject: [PATCH 06/14] net/http/pprof: run GC for /debug/pprof/heap?gc=1 We force runtime.GC before WriteHeapProfile with -test.heapprofile. Make it possible to do the same with the HTTP interface. Some servers only run a GC every few minutes. On such servers, the heap profile will be a few minutes stale, which may be too old to be useful. Requested by private mail. LGTM=dvyukov R=dvyukov CC=golang-codereviews https://golang.org/cl/161990043 --- src/net/http/pprof/pprof.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/net/http/pprof/pprof.go b/src/net/http/pprof/pprof.go index 0c7548e3ef..a23f1bc4bc 100644 --- a/src/net/http/pprof/pprof.go +++ b/src/net/http/pprof/pprof.go @@ -162,6 +162,10 @@ func (name handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { fmt.Fprintf(w, "Unknown profile: %s\n", name) return } + gc, _ := strconv.Atoi(r.FormValue("gc")) + if name == "heap" && gc > 0 { + runtime.GC() + } p.WriteTo(w, debug) return } From 1415a53b75fe3dc4fa53208e82839533bb2f1a30 Mon Sep 17 00:00:00 2001 From: Rob Pike Date: Fri, 24 Oct 2014 09:37:25 -0700 Subject: [PATCH 07/14] unsafe: document that unsafe programs are not protected The compatibility guideline needs to be clear about this even though it means adding a clause that was not there from the beginning. It has always been understood, so this isn't really a change in policy, just in its expression. LGTM=bradfitz, gri, rsc R=golang-codereviews, bradfitz, gri, rsc CC=golang-codereviews https://golang.org/cl/162060043 --- doc/go1compat.html | 8 ++++++++ src/unsafe/unsafe.go | 3 +++ 2 files changed, 11 insertions(+) diff --git a/doc/go1compat.html b/doc/go1compat.html index 04a6c1124b..94c48d2ce3 100644 --- a/doc/go1compat.html +++ b/doc/go1compat.html @@ -104,6 +104,14 @@ outside of tests, and using it may cause a program to fail to compile in future releases. + +Use of package +unsafe. Packages that import +unsafe+may depend on internal properties of the Go implementation. +We reserve the right to make changes to the implementation +that may break such programs. +diff --git a/src/unsafe/unsafe.go b/src/unsafe/unsafe.go index 83b2e14052..79499b2955 100644 --- a/src/unsafe/unsafe.go +++ b/src/unsafe/unsafe.go @@ -4,6 +4,9 @@ /* Package unsafe contains operations that step around the type safety of Go programs. + + Packages that import unsafe may be non-portable and are not protected by the + Go 1 compatibility guidelines. */ package unsafe From c2b7b6d5da9768cf35c91947157901b623778fa6 Mon Sep 17 00:00:00 2001 From: Rob Pike
Date: Fri, 24 Oct 2014 09:52:11 -0700 Subject: [PATCH 08/14] doc/go1.4.txt: unsafe is outside go1 compatibility guarantees CC=golang-codereviews https://golang.org/cl/164770043 --- doc/go1.4.txt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/doc/go1.4.txt b/doc/go1.4.txt index 78f46a3296..b9d8ade245 100644 --- a/doc/go1.4.txt +++ b/doc/go1.4.txt @@ -24,6 +24,7 @@ bufio: handling of empty tokens at EOF changed, may require scanner change (CL 1 compress/flate, compress/gzip, compress/zlib: Reset support (https://codereview.appspot.com/97140043) crypto/tls: add support for ALPN (RFC 7301) (CL 108710046) crypto/tls: support programmatic selection of server certificates (CL 107400043) +encoding/asn1: optional elements with a default value will now only be omitted if they have that value (CL 86960045) flag: it is now an error to set a flag multiple times (CL 156390043) fmt: print type *map[T]T as &map[k:v] (CL 154870043) encoding/csv: do not quote empty strings, quote \. (CL 164760043) @@ -47,6 +48,6 @@ testing: add TestMain support (CL 148770043) text/scanner: add IsIdentRune field of Scanner. (CL 108030044) text/template: allow comparison of signed and unsigned integers (CL 149780043) time: use the micro symbol (ยต (U+00B5)) to print microsecond duration (CL 105030046) -encoding/asn1: optional elements with a default value will now only be omitted if they have that value (CL 86960045) +unsafe: document the existing situation that unsafe programs are not go1-guaranteed (CL 162060043) go.sys subrepo created: http://golang.org/s/go1.4-syscall From fdf458436af89b8052489fd9ba53ef16e693d12b Mon Sep 17 00:00:00 2001 From: Gustavo Niemeyer Date: Fri, 24 Oct 2014 15:49:17 -0200 Subject: [PATCH 09/14] cmd/go: add bzr support for vcs root checking Complements the logic introduced in CL 147170043. LGTM=rsc R=rsc, gustavo CC=golang-codereviews https://golang.org/cl/147240043 --- src/cmd/go/get.go | 11 +++++++++-- src/cmd/go/vcs.go | 50 ++++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 56 insertions(+), 5 deletions(-) diff --git a/src/cmd/go/get.go b/src/cmd/go/get.go index 2640339414..b8eac5c1ef 100644 --- a/src/cmd/go/get.go +++ b/src/cmd/go/get.go @@ -272,8 +272,15 @@ func downloadPackage(p *Package) error { dir := filepath.Join(p.build.SrcRoot, rootPath) if remote, err := vcs.remoteRepo(vcs, dir); err == nil { if rr, err := repoRootForImportPath(p.ImportPath); err == nil { - if remote != rr.repo { - return fmt.Errorf("%s is from %s, should be from %s", dir, remote, rr.repo) + repo := rr.repo + if rr.vcs.resolveRepo != nil { + resolved, err := rr.vcs.resolveRepo(rr.vcs, dir, repo) + if err == nil { + repo = resolved + } + } + if remote != repo { + return fmt.Errorf("%s is from %s, should be from %s", dir, remote, repo) } } } diff --git a/src/cmd/go/vcs.go b/src/cmd/go/vcs.go index 0834a7d192..1cac613388 100644 --- a/src/cmd/go/vcs.go +++ b/src/cmd/go/vcs.go @@ -34,7 +34,8 @@ type vcsCmd struct { scheme []string pingCmd string - remoteRepo func(v *vcsCmd, rootDir string) (remoteRepo string, err error) + remoteRepo func(v *vcsCmd, rootDir string) (remoteRepo string, err error) + resolveRepo func(v *vcsCmd, rootDir, remoteRepo string) (realRepo string, err error) } // A tagCmd describes a command to list available tags @@ -164,8 +165,51 @@ var vcsBzr = &vcsCmd{ tagSyncCmd: "update -r {tag}", tagSyncDefault: "update -r revno:-1", - scheme: []string{"https", "http", "bzr", "bzr+ssh"}, - pingCmd: "info {scheme}://{repo}", + scheme: []string{"https", "http", "bzr", "bzr+ssh"}, + pingCmd: "info {scheme}://{repo}", + remoteRepo: bzrRemoteRepo, + resolveRepo: bzrResolveRepo, +} + +func bzrRemoteRepo(vcsBzr *vcsCmd, rootDir string) (remoteRepo string, err error) { + outb, err := vcsBzr.runOutput(rootDir, "config parent_location") + if err != nil { + return "", err + } + return strings.TrimSpace(string(outb)), nil +} + +func bzrResolveRepo(vcsBzr *vcsCmd, rootDir, remoteRepo string) (realRepo string, err error) { + outb, err := vcsBzr.runOutput(rootDir, "info "+remoteRepo) + if err != nil { + return "", err + } + out := string(outb) + + // Expect: + // ... + // (branch root|repository branch): + // ... + + found := false + for _, prefix := range []string{"\n branch root: ", "\n repository branch: "} { + i := strings.Index(out, prefix) + if i >= 0 { + out = out[i+len(prefix):] + found = true + break + } + } + if !found { + return "", fmt.Errorf("unable to parse output of bzr info") + } + + i := strings.Index(out, "\n") + if i < 0 { + return "", fmt.Errorf("unable to parse output of bzr info") + } + out = out[:i] + return strings.TrimSpace(string(out)), nil } // vcsSvn describes how to use Subversion. From ffa5e5f7fc51e791427468ebe00a6479191430d7 Mon Sep 17 00:00:00 2001 From: Peter Collingbourne Date: Sat, 25 Oct 2014 10:30:14 -0700 Subject: [PATCH 10/14] cmd/go: pass $CGO_LDFLAGS to linker with the "gccgo" toolchain. LGTM=iant R=iant, minux CC=golang-codereviews, golang-dev https://golang.org/cl/157460043 --- src/cmd/go/build.go | 1 + 1 file changed, 1 insertion(+) diff --git a/src/cmd/go/build.go b/src/cmd/go/build.go index 49b84709e2..79a27116a1 100644 --- a/src/cmd/go/build.go +++ b/src/cmd/go/build.go @@ -1945,6 +1945,7 @@ func (tools gccgoToolchain) ld(b *builder, p *Package, out string, allactions [] } ldflags = append(ldflags, afiles...) ldflags = append(ldflags, cgoldflags...) + ldflags = append(ldflags, envList("CGO_LDFLAGS", "")...) ldflags = append(ldflags, p.CgoLDFLAGS...) if usesCgo && goos == "linux" { ldflags = append(ldflags, "-Wl,-E") From a9422651f9600c38b3f31f08f1be5a96cb338306 Mon Sep 17 00:00:00 2001 From: Rob Pike Date: Sun, 26 Oct 2014 11:27:55 -0700 Subject: [PATCH 11/14] doc/go_faq.html: fix a couple of nits Wrong article, one stylistic point that bothers someone (but not me). LGTM=bradfitz R=golang-codereviews, bradfitz CC=golang-codereviews https://golang.org/cl/156680043 --- doc/go_faq.html | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/go_faq.html b/doc/go_faq.html index ec3689aeb0..9aac058388 100644 --- a/doc/go_faq.html +++ b/doc/go_faq.html @@ -1115,7 +1115,7 @@ error but the situation can still be confusing, because sometimes a pointer is necessary to satisfy an interface. The insight is that although a pointer to a concrete type can satisfy -an interface, with one exception a pointer to an interface can never satisfy a interface. +an interface, with one exception a pointer to an interface can never satisfy an interface. @@ -1356,7 +1356,7 @@ to speed it up.
-Go's goroutine scheduler is not as good as it needs to be. In future, it +Go's goroutine scheduler is not as good as it needs to be. In the future, it should recognize such cases and optimize its use of OS threads. For now,
From 78082dfa801d484848ac47c04ce3aa9805d2b0c9 Mon Sep 17 00:00:00 2001 From: Emil HessmanGOMAXPROCSshould be set on a per-application basis.Date: Mon, 27 Oct 2014 12:43:14 +1100 Subject: [PATCH 12/14] misc/makerelease/windows: fix 404 help URL in installer ARPHELPLINK yields 404; update the URL. While here, also prefix the ARPREADME and ARPURLINFOABOUT URL's with the HTTP scheme to make 'em clickable links in the Add or Remove Programs listing. LGTM=adg R=golang-codereviews CC=adg, golang-codereviews https://golang.org/cl/154580045 --- misc/makerelease/windows/installer.wxs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/misc/makerelease/windows/installer.wxs b/misc/makerelease/windows/installer.wxs index 66e0913ba8..01178e2651 100644 --- a/misc/makerelease/windows/installer.wxs +++ b/misc/makerelease/windows/installer.wxs @@ -39,9 +39,9 @@ - - - + + + 1 From 77595e462be07b8229f88cbdf947e320bfc7e639 Mon Sep 17 00:00:00 2001 From: Ian Lance Taylor Date: Mon, 27 Oct 2014 08:46:18 -0700 Subject: [PATCH 13/14] net: if a DNS lookup times out, forget that it is in flight Before this CL, if the system resolver does a very slow DNS lookup for a particular host, all subsequent requests for that host will hang waiting for that lookup to complete. That is more or less expected when Dial is called with no deadline. When Dial has a deadline, though, we can accumulate a large number of goroutines waiting for that slow DNS lookup. Try to avoid this problem by restarting the DNS lookup when it is redone after a deadline is passed. This CL also avoids creating an extra goroutine purely to handle the deadline. No test because we would have to simulate a slow DNS lookup followed by a fast DNS lookup. Fixes #8602. LGTM=bradfitz R=bradfitz, mikioh.mikioh CC=golang-codereviews, r, rsc https://golang.org/cl/154610044 --- src/net/lookup.go | 51 +++++++++++++++++-------------- src/net/singleflight.go | 68 +++++++++++++++++++++++++++++++++++++---- 2 files changed, 90 insertions(+), 29 deletions(-) diff --git a/src/net/lookup.go b/src/net/lookup.go index 20f20578cd..aeffe6c9b7 100644 --- a/src/net/lookup.go +++ b/src/net/lookup.go @@ -40,10 +40,16 @@ func lookupIPMerge(host string) (addrs []IP, err error) { addrsi, err, shared := lookupGroup.Do(host, func() (interface{}, error) { return lookupIP(host) }) + return lookupIPReturn(addrsi, err, shared) +} + +// lookupIPReturn turns the return values from singleflight.Do into +// the return values from LookupIP. +func lookupIPReturn(addrsi interface{}, err error, shared bool) ([]IP, error) { if err != nil { return nil, err } - addrs = addrsi.([]IP) + addrs := addrsi.([]IP) if shared { clone := make([]IP, len(addrs)) copy(clone, addrs) @@ -52,41 +58,40 @@ func lookupIPMerge(host string) (addrs []IP, err error) { return addrs, nil } +// lookupIPDeadline looks up a hostname with a deadline. func lookupIPDeadline(host string, deadline time.Time) (addrs []IP, err error) { if deadline.IsZero() { return lookupIPMerge(host) } - // TODO(bradfitz): consider pushing the deadline down into the - // name resolution functions. But that involves fixing it for - // the native Go resolver, cgo, Windows, etc. - // - // In the meantime, just use a goroutine. Most users affected - // by http://golang.org/issue/2631 are due to TCP connections - // to unresponsive hosts, not DNS. + // We could push the deadline down into the name resolution + // functions. However, the most commonly used implementation + // calls getaddrinfo, which has no timeout. + timeout := deadline.Sub(time.Now()) if timeout <= 0 { - err = errTimeout - return + return nil, errTimeout } t := time.NewTimer(timeout) defer t.Stop() - type res struct { - addrs []IP - err error - } - resc := make(chan res, 1) - go func() { - a, err := lookupIPMerge(host) - resc <- res{a, err} - }() + + ch := lookupGroup.DoChan(host, func() (interface{}, error) { + return lookupIP(host) + }) + select { case <-t.C: - err = errTimeout - case r := <-resc: - addrs, err = r.addrs, r.err + // The DNS lookup timed out for some reason. Force + // future requests to start the DNS lookup again + // rather than waiting for the current lookup to + // complete. See issue 8602. + lookupGroup.Forget(host) + + return nil, errTimeout + + case r := <-ch: + return lookupIPReturn(r.v, r.err, r.shared) } - return } // LookupPort looks up the port for the given network and service. diff --git a/src/net/singleflight.go b/src/net/singleflight.go index dc58affdaa..bf599f0cc9 100644 --- a/src/net/singleflight.go +++ b/src/net/singleflight.go @@ -8,10 +8,18 @@ import "sync" // call is an in-flight or completed singleflight.Do call type call struct { - wg sync.WaitGroup - val interface{} - err error - dups int + wg sync.WaitGroup + + // These fields are written once before the WaitGroup is done + // and are only read after the WaitGroup is done. + val interface{} + err error + + // These fields are read and written with the singleflight + // mutex held before the WaitGroup is done, and are read but + // not written after the WaitGroup is done. + dups int + chans []chan<- singleflightResult } // singleflight represents a class of work and forms a namespace in @@ -21,6 +29,14 @@ type singleflight struct { m map[string]*call // lazily initialized } +// singleflightResult holds the results of Do, so they can be passed +// on a channel. +type singleflightResult struct { + v interface{} + err error + shared bool +} + // Do executes and returns the results of the given function, making // sure that only one execution is in-flight for a given key at a // time. If a duplicate comes in, the duplicate caller waits for the @@ -42,12 +58,52 @@ func (g *singleflight) Do(key string, fn func() (interface{}, error)) (v interfa g.m[key] = c g.mu.Unlock() + g.doCall(c, key, fn) + return c.val, c.err, c.dups > 0 +} + +// DoChan is like Do but returns a channel that will receive the +// results when they are ready. +func (g *singleflight) DoChan(key string, fn func() (interface{}, error)) <-chan singleflightResult { + ch := make(chan singleflightResult, 1) + g.mu.Lock() + if g.m == nil { + g.m = make(map[string]*call) + } + if c, ok := g.m[key]; ok { + c.dups++ + c.chans = append(c.chans, ch) + g.mu.Unlock() + return ch + } + c := &call{chans: []chan<- singleflightResult{ch}} + c.wg.Add(1) + g.m[key] = c + g.mu.Unlock() + + go g.doCall(c, key, fn) + + return ch +} + +// doCall handles the single call for a key. +func (g *singleflight) doCall(c *call, key string, fn func() (interface{}, error)) { c.val, c.err = fn() c.wg.Done() + g.mu.Lock() + delete(g.m, key) + for _, ch := range c.chans { + ch <- singleflightResult{c.val, c.err, c.dups > 0} + } + g.mu.Unlock() +} + +// Forget tells the singleflight to forget about a key. Future calls +// to Do for this key will call the function rather than waiting for +// an earlier call to complete. +func (g *singleflight) Forget(key string) { g.mu.Lock() delete(g.m, key) g.mu.Unlock() - - return c.val, c.err, c.dups > 0 } From 3e62d2184ab2d2ac6053e3f4af5e3f99902c1e32 Mon Sep 17 00:00:00 2001 From: Austin Clements Date: Mon, 27 Oct 2014 17:12:48 -0400 Subject: [PATCH 14/14] runtime: fix endianness assumption when decoding ftab The ftab ends with a half functab record consisting only of the 'entry' field followed by a uint32 giving the offset of the next table. Previously, symtabinit assumed it could read this uint32 as a uintptr. Since this is unsafe on big endian, explicitly read the offset as a uint32. LGTM=rsc R=rsc CC=golang-codereviews https://golang.org/cl/157660043 --- src/runtime/symtab.go | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/runtime/symtab.go b/src/runtime/symtab.go index 48d4023b9a..45d107b777 100644 --- a/src/runtime/symtab.go +++ b/src/runtime/symtab.go @@ -84,10 +84,13 @@ func symtabinit() { } } - // file table follows ftab. + // The ftab ends with a half functab consisting only of + // 'entry', followed by a uint32 giving the pcln-relative + // offset of the file table. sp = (*sliceStruct)(unsafe.Pointer(&filetab)) - p = unsafe.Pointer(add(unsafe.Pointer(pcln), ftab[nftab].funcoff)) - sp.array = unsafe.Pointer(add(unsafe.Pointer(pcln), ftab[nftab].funcoff)) + end := unsafe.Pointer(&ftab[nftab].funcoff) // just beyond ftab + fileoffset := *(*uint32)(end) + sp.array = unsafe.Pointer(&pclntable[fileoffset]) // length is in first element of array. // set len to 1 so we can get first element. sp.len = 1 @@ -224,7 +227,7 @@ func funcline(f *_func, targetpc uintptr, file *string) int32 { func funcspdelta(f *_func, targetpc uintptr) int32 { x := pcvalue(f, f.pcsp, targetpc, true) if x&(ptrSize-1) != 0 { - print("invalid spdelta ", f.pcsp, " ", x, "\n") + print("invalid spdelta ", hex(f.entry), " ", hex(targetpc), " ", hex(f.pcsp), " ", x, "\n") } return x }