diff --git a/libgo/MERGE b/libgo/MERGE index 92541dfc6e3..fa155a238d6 100644 --- a/libgo/MERGE +++ b/libgo/MERGE @@ -1,4 +1,4 @@ -59f181b6fda68ece22882945853ca2df9dbf1c88 +2a5f65a98ca483aad2dd74dc2636a7baecc59cf2 The first line of this file holds the git revision number of the last merge done from the master library sources. diff --git a/libgo/VERSION b/libgo/VERSION index 212211f54d1..8b33af3b6b0 100644 --- a/libgo/VERSION +++ b/libgo/VERSION @@ -1 +1 @@ -go1.8rc2 +go1.8rc3 diff --git a/libgo/go/cmd/go/go_test.go b/libgo/go/cmd/go/go_test.go index 5727eb094e5..f26c3660e4b 100644 --- a/libgo/go/cmd/go/go_test.go +++ b/libgo/go/cmd/go/go_test.go @@ -3787,3 +3787,26 @@ GLOBL ·constants<>(SB),8,$8 tg.setenv("GOPATH", tg.path("go")) tg.run("build", "p") } + +// Issue 18778. +func TestDotDotDotOutsideGOPATH(t *testing.T) { + tg := testgo(t) + defer tg.cleanup() + + tg.tempFile("pkgs/a.go", `package x`) + tg.tempFile("pkgs/a_test.go", `package x_test +import "testing" +func TestX(t *testing.T) {}`) + + tg.tempFile("pkgs/a/a.go", `package a`) + tg.tempFile("pkgs/a/a_test.go", `package a_test +import "testing" +func TestA(t *testing.T) {}`) + + tg.cd(tg.path("pkgs")) + tg.run("build", "./...") + tg.run("test", "./...") + tg.run("list", "./...") + tg.grepStdout("pkgs$", "expected package not listed") + tg.grepStdout("pkgs/a", "expected package not listed") +} diff --git a/libgo/go/cmd/go/pkg.go b/libgo/go/cmd/go/pkg.go index af8c1d959fa..61c1b9960ea 100644 --- a/libgo/go/cmd/go/pkg.go +++ b/libgo/go/cmd/go/pkg.go @@ -433,7 +433,7 @@ func setErrorPos(p *Package, importPos []token.Position) *Package { func cleanImport(path string) string { orig := path path = pathpkg.Clean(path) - if strings.HasPrefix(orig, "./") && path != ".." && path != "." && !strings.HasPrefix(path, "../") { + if strings.HasPrefix(orig, "./") && path != ".." && !strings.HasPrefix(path, "../") { path = "./" + path } return path diff --git a/libgo/go/database/sql/sql.go b/libgo/go/database/sql/sql.go index 0fa7c34a13e..feb91223a9e 100644 --- a/libgo/go/database/sql/sql.go +++ b/libgo/go/database/sql/sql.go @@ -1357,16 +1357,7 @@ func (db *DB) begin(ctx context.Context, opts *TxOptions, strategy connReuseStra cancel: cancel, ctx: ctx, } - go func(tx *Tx) { - select { - case <-tx.ctx.Done(): - if !tx.isDone() { - // Discard and close the connection used to ensure the transaction - // is closed and the resources are released. - tx.rollback(true) - } - } - }(tx) + go tx.awaitDone() return tx, nil } @@ -1388,6 +1379,11 @@ func (db *DB) Driver() driver.Driver { type Tx struct { db *DB + // closemu prevents the transaction from closing while there + // is an active query. It is held for read during queries + // and exclusively during close. + closemu sync.RWMutex + // dc is owned exclusively until Commit or Rollback, at which point // it's returned with putConn. dc *driverConn @@ -1413,6 +1409,20 @@ type Tx struct { ctx context.Context } +// awaitDone blocks until the context in Tx is canceled and rolls back +// the transaction if it's not already done. +func (tx *Tx) awaitDone() { + // Wait for either the transaction to be committed or rolled + // back, or for the associated context to be closed. + <-tx.ctx.Done() + + // Discard and close the connection used to ensure the + // transaction is closed and the resources are released. This + // rollback does nothing if the transaction has already been + // committed or rolled back. + tx.rollback(true) +} + func (tx *Tx) isDone() bool { return atomic.LoadInt32(&tx.done) != 0 } @@ -1424,16 +1434,31 @@ var ErrTxDone = errors.New("sql: Transaction has already been committed or rolle // close returns the connection to the pool and // must only be called by Tx.rollback or Tx.Commit. func (tx *Tx) close(err error) { + tx.closemu.Lock() + defer tx.closemu.Unlock() + tx.db.putConn(tx.dc, err) tx.cancel() tx.dc = nil tx.txi = nil } +// hookTxGrabConn specifies an optional hook to be called on +// a successful call to (*Tx).grabConn. For tests. +var hookTxGrabConn func() + func (tx *Tx) grabConn(ctx context.Context) (*driverConn, error) { + select { + default: + case <-ctx.Done(): + return nil, ctx.Err() + } if tx.isDone() { return nil, ErrTxDone } + if hookTxGrabConn != nil { // test hook + hookTxGrabConn() + } return tx.dc, nil } @@ -1503,6 +1528,9 @@ func (tx *Tx) Rollback() error { // for the execution of the returned statement. The returned statement // will run in the transaction context. func (tx *Tx) PrepareContext(ctx context.Context, query string) (*Stmt, error) { + tx.closemu.RLock() + defer tx.closemu.RUnlock() + // TODO(bradfitz): We could be more efficient here and either // provide a method to take an existing Stmt (created on // perhaps a different Conn), and re-create it on this Conn if @@ -1567,6 +1595,9 @@ func (tx *Tx) Prepare(query string) (*Stmt, error) { // The returned statement operates within the transaction and will be closed // when the transaction has been committed or rolled back. func (tx *Tx) StmtContext(ctx context.Context, stmt *Stmt) *Stmt { + tx.closemu.RLock() + defer tx.closemu.RUnlock() + // TODO(bradfitz): optimize this. Currently this re-prepares // each time. This is fine for now to illustrate the API but // we should really cache already-prepared statements @@ -1618,6 +1649,9 @@ func (tx *Tx) Stmt(stmt *Stmt) *Stmt { // ExecContext executes a query that doesn't return rows. // For example: an INSERT and UPDATE. func (tx *Tx) ExecContext(ctx context.Context, query string, args ...interface{}) (Result, error) { + tx.closemu.RLock() + defer tx.closemu.RUnlock() + dc, err := tx.grabConn(ctx) if err != nil { return nil, err @@ -1661,6 +1695,9 @@ func (tx *Tx) Exec(query string, args ...interface{}) (Result, error) { // QueryContext executes a query that returns rows, typically a SELECT. func (tx *Tx) QueryContext(ctx context.Context, query string, args ...interface{}) (*Rows, error) { + tx.closemu.RLock() + defer tx.closemu.RUnlock() + dc, err := tx.grabConn(ctx) if err != nil { return nil, err @@ -2038,25 +2075,21 @@ type Rows struct { // closed value is 1 when the Rows is closed. // Use atomic operations on value when checking value. closed int32 - ctxClose chan struct{} // closed when Rows is closed, may be null. + cancel func() // called when Rows is closed, may be nil. lastcols []driver.Value lasterr error // non-nil only if closed is true closeStmt *driverStmt // if non-nil, statement to Close on close } func (rs *Rows) initContextClose(ctx context.Context) { - if ctx.Done() == context.Background().Done() { - return - } + ctx, rs.cancel = context.WithCancel(ctx) + go rs.awaitDone(ctx) +} - rs.ctxClose = make(chan struct{}) - go func() { - select { - case <-ctx.Done(): - rs.Close() - case <-rs.ctxClose: - } - }() +// awaitDone blocks until the rows are closed or the context canceled. +func (rs *Rows) awaitDone(ctx context.Context) { + <-ctx.Done() + rs.Close() } // Next prepares the next result row for reading with the Scan method. It @@ -2314,7 +2347,9 @@ func (rs *Rows) Scan(dest ...interface{}) error { return nil } -var rowsCloseHook func(*Rows, *error) +// rowsCloseHook returns a function so tests may install the +// hook throug a test only mutex. +var rowsCloseHook = func() func(*Rows, *error) { return nil } func (rs *Rows) isClosed() bool { return atomic.LoadInt32(&rs.closed) != 0 @@ -2328,13 +2363,15 @@ func (rs *Rows) Close() error { if !atomic.CompareAndSwapInt32(&rs.closed, 0, 1) { return nil } - if rs.ctxClose != nil { - close(rs.ctxClose) - } + err := rs.rowsi.Close() - if fn := rowsCloseHook; fn != nil { + if fn := rowsCloseHook(); fn != nil { fn(rs, &err) } + if rs.cancel != nil { + rs.cancel() + } + if rs.closeStmt != nil { rs.closeStmt.Close() } diff --git a/libgo/go/database/sql/sql_test.go b/libgo/go/database/sql/sql_test.go index 63e1292cb1f..898df3b455b 100644 --- a/libgo/go/database/sql/sql_test.go +++ b/libgo/go/database/sql/sql_test.go @@ -14,6 +14,7 @@ import ( "runtime" "strings" "sync" + "sync/atomic" "testing" "time" ) @@ -326,9 +327,7 @@ func TestQueryContext(t *testing.T) { // And verify that the final rows.Next() call, which hit EOF, // also closed the rows connection. - if n := db.numFreeConns(); n != 1 { - t.Fatalf("free conns after query hitting EOF = %d; want 1", n) - } + waitForFree(t, db, 5*time.Second, 1) if prepares := numPrepares(t, db) - prepares0; prepares != 1 { t.Errorf("executed %d Prepare statements; want 1", prepares) } @@ -345,6 +344,18 @@ func waitCondition(waitFor, checkEvery time.Duration, fn func() bool) bool { return false } +// waitForFree checks db.numFreeConns until either it equals want or +// the maxWait time elapses. +func waitForFree(t *testing.T, db *DB, maxWait time.Duration, want int) { + var numFree int + if !waitCondition(maxWait, 5*time.Millisecond, func() bool { + numFree = db.numFreeConns() + return numFree == want + }) { + t.Fatalf("free conns after hitting EOF = %d; want %d", numFree, want) + } +} + func TestQueryContextWait(t *testing.T) { db := newTestDB(t, "people") defer closeDB(t, db) @@ -361,9 +372,7 @@ func TestQueryContextWait(t *testing.T) { } // Verify closed rows connection after error condition. - if n := db.numFreeConns(); n != 1 { - t.Fatalf("free conns after query hitting EOF = %d; want 1", n) - } + waitForFree(t, db, 5*time.Second, 1) if prepares := numPrepares(t, db) - prepares0; prepares != 1 { t.Errorf("executed %d Prepare statements; want 1", prepares) } @@ -388,13 +397,7 @@ func TestTxContextWait(t *testing.T) { t.Fatalf("expected QueryContext to error with context deadline exceeded but returned %v", err) } - var numFree int - if !waitCondition(5*time.Second, 5*time.Millisecond, func() bool { - numFree = db.numFreeConns() - return numFree == 0 - }) { - t.Fatalf("free conns after hitting EOF = %d; want 0", numFree) - } + waitForFree(t, db, 5*time.Second, 0) // Ensure the dropped connection allows more connections to be made. // Checked on DB Close. @@ -471,9 +474,7 @@ func TestMultiResultSetQuery(t *testing.T) { // And verify that the final rows.Next() call, which hit EOF, // also closed the rows connection. - if n := db.numFreeConns(); n != 1 { - t.Fatalf("free conns after query hitting EOF = %d; want 1", n) - } + waitForFree(t, db, 5*time.Second, 1) if prepares := numPrepares(t, db) - prepares0; prepares != 1 { t.Errorf("executed %d Prepare statements; want 1", prepares) } @@ -1135,6 +1136,24 @@ func TestQueryRowClosingStmt(t *testing.T) { } } +var atomicRowsCloseHook atomic.Value // of func(*Rows, *error) + +func init() { + rowsCloseHook = func() func(*Rows, *error) { + fn, _ := atomicRowsCloseHook.Load().(func(*Rows, *error)) + return fn + } +} + +func setRowsCloseHook(fn func(*Rows, *error)) { + if fn == nil { + // Can't change an atomic.Value back to nil, so set it to this + // no-op func instead. + fn = func(*Rows, *error) {} + } + atomicRowsCloseHook.Store(fn) +} + // Test issue 6651 func TestIssue6651(t *testing.T) { db := newTestDB(t, "people") @@ -1147,6 +1166,7 @@ func TestIssue6651(t *testing.T) { return fmt.Errorf(want) } defer func() { rowsCursorNextHook = nil }() + err := db.QueryRow("SELECT|people|name|").Scan(&v) if err == nil || err.Error() != want { t.Errorf("error = %q; want %q", err, want) @@ -1154,10 +1174,10 @@ func TestIssue6651(t *testing.T) { rowsCursorNextHook = nil want = "error in rows.Close" - rowsCloseHook = func(rows *Rows, err *error) { + setRowsCloseHook(func(rows *Rows, err *error) { *err = fmt.Errorf(want) - } - defer func() { rowsCloseHook = nil }() + }) + defer setRowsCloseHook(nil) err = db.QueryRow("SELECT|people|name|").Scan(&v) if err == nil || err.Error() != want { t.Errorf("error = %q; want %q", err, want) @@ -1830,7 +1850,9 @@ func TestStmtCloseDeps(t *testing.T) { db.dumpDeps(t) } - if len(stmt.css) > nquery { + if !waitCondition(5*time.Second, 5*time.Millisecond, func() bool { + return len(stmt.css) <= nquery + }) { t.Errorf("len(stmt.css) = %d; want <= %d", len(stmt.css), nquery) } @@ -2576,10 +2598,10 @@ func TestIssue6081(t *testing.T) { if err != nil { t.Fatal(err) } - rowsCloseHook = func(rows *Rows, err *error) { + setRowsCloseHook(func(rows *Rows, err *error) { *err = driver.ErrBadConn - } - defer func() { rowsCloseHook = nil }() + }) + defer setRowsCloseHook(nil) for i := 0; i < 10; i++ { rows, err := stmt.Query() if err != nil { @@ -2642,7 +2664,10 @@ func TestIssue18429(t *testing.T) { if err != nil { return } - rows, err := tx.QueryContext(ctx, "WAIT|"+qwait+"|SELECT|people|name|") + // This is expected to give a cancel error many, but not all the time. + // Test failure will happen with a panic or other race condition being + // reported. + rows, _ := tx.QueryContext(ctx, "WAIT|"+qwait+"|SELECT|people|name|") if rows != nil { rows.Close() } @@ -2655,6 +2680,56 @@ func TestIssue18429(t *testing.T) { time.Sleep(milliWait * 3 * time.Millisecond) } +// TestIssue18719 closes the context right before use. The sql.driverConn +// will nil out the ci on close in a lock, but if another process uses it right after +// it will panic with on the nil ref. +// +// See https://golang.org/cl/35550 . +func TestIssue18719(t *testing.T) { + db := newTestDB(t, "people") + defer closeDB(t, db) + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + tx, err := db.BeginTx(ctx, nil) + if err != nil { + t.Fatal(err) + } + + hookTxGrabConn = func() { + cancel() + + // Wait for the context to cancel and tx to rollback. + for tx.isDone() == false { + time.Sleep(time.Millisecond * 3) + } + } + defer func() { hookTxGrabConn = nil }() + + // This call will grab the connection and cancel the context + // after it has done so. Code after must deal with the canceled state. + rows, err := tx.QueryContext(ctx, "SELECT|people|name|") + if err != nil { + rows.Close() + t.Fatalf("expected error %v but got %v", nil, err) + } + + // Rows may be ignored because it will be closed when the context is canceled. + + // Do not explicitly rollback. The rollback will happen from the + // canceled context. + + // Wait for connections to return to pool. + var numOpen int + if !waitCondition(5*time.Second, 5*time.Millisecond, func() bool { + numOpen = db.numOpenConns() + return numOpen == 0 + }) { + t.Fatalf("open conns after hitting EOF = %d; want 0", numOpen) + } +} + func TestConcurrency(t *testing.T) { doConcurrentTest(t, new(concurrentDBQueryTest)) doConcurrentTest(t, new(concurrentDBExecTest)) diff --git a/libgo/go/go/printer/nodes.go b/libgo/go/go/printer/nodes.go index 11f26d45ea3..ea432860a8e 100644 --- a/libgo/go/go/printer/nodes.go +++ b/libgo/go/go/printer/nodes.go @@ -733,7 +733,7 @@ func (p *printer) expr1(expr ast.Expr, prec1, depth int) { case *ast.FuncLit: p.expr(x.Type) - p.adjBlock(p.distanceFrom(x.Type.Pos()), blank, x.Body) + p.funcBody(p.distanceFrom(x.Type.Pos()), blank, x.Body) case *ast.ParenExpr: if _, hasParens := x.X.(*ast.ParenExpr); hasParens { @@ -825,6 +825,7 @@ func (p *printer) expr1(expr ast.Expr, prec1, depth int) { if x.Type != nil { p.expr1(x.Type, token.HighestPrec, depth) } + p.level++ p.print(x.Lbrace, token.LBRACE) p.exprList(x.Lbrace, x.Elts, 1, commaTerm, x.Rbrace) // do not insert extra line break following a /*-style comment @@ -837,6 +838,7 @@ func (p *printer) expr1(expr ast.Expr, prec1, depth int) { mode |= noExtraBlank } p.print(mode, x.Rbrace, token.RBRACE, mode) + p.level-- case *ast.Ellipsis: p.print(token.ELLIPSIS) @@ -1557,18 +1559,23 @@ func (p *printer) bodySize(b *ast.BlockStmt, maxSize int) int { return bodySize } -// adjBlock prints an "adjacent" block (e.g., a for-loop or function body) following -// a header (e.g., a for-loop control clause or function signature) of given headerSize. +// funcBody prints a function body following a function header of given headerSize. // If the header's and block's size are "small enough" and the block is "simple enough", // the block is printed on the current line, without line breaks, spaced from the header // by sep. Otherwise the block's opening "{" is printed on the current line, followed by // lines for the block's statements and its closing "}". // -func (p *printer) adjBlock(headerSize int, sep whiteSpace, b *ast.BlockStmt) { +func (p *printer) funcBody(headerSize int, sep whiteSpace, b *ast.BlockStmt) { if b == nil { return } + // save/restore composite literal nesting level + defer func(level int) { + p.level = level + }(p.level) + p.level = 0 + const maxSize = 100 if headerSize+p.bodySize(b, maxSize) <= maxSize { p.print(sep, b.Lbrace, token.LBRACE) @@ -1613,7 +1620,7 @@ func (p *printer) funcDecl(d *ast.FuncDecl) { } p.expr(d.Name) p.signature(d.Type.Params, d.Type.Results) - p.adjBlock(p.distanceFrom(d.Pos()), vtab, d.Body) + p.funcBody(p.distanceFrom(d.Pos()), vtab, d.Body) } func (p *printer) decl(decl ast.Decl) { diff --git a/libgo/go/go/printer/printer.go b/libgo/go/go/printer/printer.go index eabf23e8b28..be61dad590e 100644 --- a/libgo/go/go/printer/printer.go +++ b/libgo/go/go/printer/printer.go @@ -58,6 +58,7 @@ type printer struct { // Current state output []byte // raw printer result indent int // current indentation + level int // level == 0: outside composite literal; level > 0: inside composite literal mode pmode // current printer mode impliedSemi bool // if set, a linebreak implies a semicolon lastTok token.Token // last token printed (token.ILLEGAL if it's whitespace) @@ -744,15 +745,19 @@ func (p *printer) intersperseComments(next token.Position, tok token.Token) (wro // follows on the same line but is not a comma, and not a "closing" // token immediately following its corresponding "opening" token, // add an extra separator unless explicitly disabled. Use a blank - // as separator unless we have pending linebreaks and they are not - // disabled, in which case we want a linebreak (issue 15137). + // as separator unless we have pending linebreaks, they are not + // disabled, and we are outside a composite literal, in which case + // we want a linebreak (issue 15137). + // TODO(gri) This has become overly complicated. We should be able + // to track whether we're inside an expression or statement and + // use that information to decide more directly. needsLinebreak := false if p.mode&noExtraBlank == 0 && last.Text[1] == '*' && p.lineFor(last.Pos()) == next.Line && tok != token.COMMA && (tok != token.RPAREN || p.prevOpen == token.LPAREN) && (tok != token.RBRACK || p.prevOpen == token.LBRACK) { - if p.containsLinebreak() && p.mode&noExtraLinebreak == 0 { + if p.containsLinebreak() && p.mode&noExtraLinebreak == 0 && p.level == 0 { needsLinebreak = true } else { p.writeByte(' ', 1) diff --git a/libgo/go/go/printer/testdata/comments2.golden b/libgo/go/go/printer/testdata/comments2.golden index 7676a26c125..8b3a94ddcd0 100644 --- a/libgo/go/go/printer/testdata/comments2.golden +++ b/libgo/go/go/printer/testdata/comments2.golden @@ -103,3 +103,62 @@ label: mask := uint64(1)<