runtime: fatal error: sweep increased allocation count, go1.9.x

[pciet]

What version of Go are you using (go version)?

go version go1.9.2 linux/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

Ubuntu Server 16.04.3 LTS with PostgreSQL 9.5.10 and OpenSSH_7.2p2 selected during install.

Intel(R) Core(TM) i5-6400 CPU

GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/pciet/go"
GORACE=""
GOROOT="/usr/local/go"
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0"
CXX="g++"
CGO_ENABLED="1"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"

What did you do?

Load testing by acting as 40 clients automatically without delays between actions playing my chess game implemented as a Go HTTP server with a postgres database, which should partially simulate a load of hundreds of players. Crash within 30 seconds as the number of active clients ramps up. The clients are making HTTP calls from a separate 2015 MacBook Pro on the local network.

What did you expect to see?

The load test should run indefinitely.

What did you see instead?

Crash with the signature "fatal error: sweep increased allocation count" reproducible within 30 seconds. The crash does not happen with a client count of eight or less. The crash does not happen with go1.6.4, go1.7.6, or go1.8.4, but does with go1.9, go1.9.1, and go1.9.2.

A race condition is warned with go build -race set, but the memory indicated should not have been shared between goroutines.

My investigation is detailed here, with stack traces (with and without GODEBUG=gccheckmark=1) and the race warnings: https://github.com/pciet/wichess/issues/19

My email to golang-nuts: https://groups.google.com/forum/#!topic/golang-nuts/7lcongdGOMM

I'll try to put together a case with less code and without the database.

[ALTree]

A race condition is warned with go build -race set, but the memory indicated should not have been shared between goroutines.

If the code triggers a race condition warning, then there's a race condition in the code and the program is not valid. Can you explain why do you believe that the reported race is not actually a race? Are you suggesting there's a bug in the race detector?

[bradfitz]

If the code triggers a race condition warning, then there's a race condition in the code and the program is not valid.

Nevertheless, Go tries not to crash just because there's a data race. (Go doesn't makes as many promises as Java in the face of a data race, but it does try not to crash.)

[pciet]

Read at 0x00c4201c4ef0 by goroutine 125:
  github.com/pciet/wichess/wichessing.Board.MovesFromPoint()
      /home/pciet/go/src/github.com/pciet/wichess/wichessing/board_moves.go:216 +0x538
...
Previous write at 0x00c4201c4ef0 by goroutine 49:
  [failed to restore the stack]

214	for movetype, unfilteredpaths := range TruncatedAbsPathsForKind(the.Piece.Kind, the.AbsPoint, the.Piece.Orientation) {
215		for path, _ := range b.ActualPaths(the, movetype, unfilteredpaths) {
216			for _, pt := range path.Points {
...
384 func (b Board) ActualPaths(the Point, movetype PathType, unfilteredpaths AbsPathSet) AbsPathSet {
...
388	actualPaths := make(AbsPathSet)
...
398			filteredPath := AbsPath{
399				Points: make([]AbsPoint, 0, len(path.Points)),
400			}
...
437				filteredPath.Points = append(filteredPath.Points, point)
438			}
439			if len(filteredPath.Points) > 0 {
440				actualPaths[filteredPath.Copy()] = struct{}{}
441			}

These path datatypes represent the moves a chess piece can make on the board. The function TruncatedAbsPathsForKind translates predefined move patterns relative from zero for a specific piece into a set of AbsPath made of AbsPoint (absolute path, absolute point) that fits in the range 0-7 (truncated means the edge of the board was detected before the RelPath could be completed).

Those path generating functions are called multiple times in these Board methods:

// Moves, Check, Checkmate.
func (b Board) Moves(active Orientation, previousFrom AbsPoint, previousTo AbsPoint) (map[AbsPoint]AbsPointSet, bool, bool) {
...
func (b Board) Checkmate(turn Orientation, previousFrom AbsPoint, previousTo AbsPoint) bool {
...
func (b Board) Check(turn Orientation, previousFrom AbsPoint, previousTo AbsPoint) bool {
...
func (b Board) Draw(turn Orientation, previousFrom AbsPoint, previousTo AbsPoint) bool {
...
func (b Board) AfterMove(from AbsPoint, to AbsPoint, turn Orientation, previousFrom AbsPoint, previousTo AbsPoint) Board {
...
// Diff, Taken Pieces
func (b Board) Move(from AbsPoint, to AbsPoint, turn Orientation, previousFrom AbsPoint, previousTo AbsPoint) (PointSet, map[AbsPoint]*Piece) {

The application code (such as what happens when a player requests what moves are currently available for all pieces via a HTTP GET request) calls into these methods after reading the latest representation of the board from the database without sharing these data structures - each request is one goroutine, and the only shared memory is the postgres database and game state diffs sent to a websocket goroutine for each player currently on the game page. This shared memory for sending diffs is not pointed to in these race conditions.

==================
WARNING: DATA RACE
Read at 0x00c4205df498 by goroutine 49:
  github.com/pciet/wichess/wichessing.Board.ActualPaths()
      /home/pciet/go/src/github.com/pciet/wichess/wichessing/board_moves.go:395 +0xde3
  github.com/pciet/wichess/wichessing.Board.MovesFromPoint()
      /home/pciet/go/src/github.com/pciet/wichess/wichessing/board_moves.go:215 +0x9bd
  github.com/pciet/wichess/wichessing.Board.Move()
      /home/pciet/go/src/github.com/pciet/wichess/wichessing/board_move.go:126 +0x12b1
  github.com/pciet/wichess/wichessing.Board.AfterMove()
      /home/pciet/go/src/github.com/pciet/wichess/wichessing/board_move.go:10 +0x18a
  github.com/pciet/wichess/wichessing.Board.Check()
      /home/pciet/go/src/github.com/pciet/wichess/wichessing/board_check.go:47 +0x2ae
  github.com/pciet/wichess/wichessing.Board.Moves()
      /home/pciet/go/src/github.com/pciet/wichess/wichessing/board_moves.go:50 +0x5ca
  main.game.moves()
      /home/pciet/go/src/github.com/pciet/wichess/game_moving.go:266 +0x862
  main.movesHandler()
      /home/pciet/go/src/github.com/pciet/wichess/web_moves.go:55 +0x4d0
  net/http.HandlerFunc.ServeHTTP()
      /usr/local/go/src/net/http/server.go:1918 +0x51
  net/http.(*ServeMux).ServeHTTP()
      /usr/local/go/src/net/http/server.go:2254 +0xa2
  net/http.serverHandler.ServeHTTP()
      /usr/local/go/src/net/http/server.go:2619 +0xbc
  net/http.(*conn).serve()
      /usr/local/go/src/net/http/server.go:1801 +0x83b

Previous write at 0x00c4205df498 by goroutine 125:
  github.com/pciet/wichess/wichessing.TruncatedAbsPathsForKind()
      /home/pciet/go/src/github.com/pciet/wichess/wichessing/path.go:19 +0x3d2
  github.com/pciet/wichess/wichessing.Board.MovesFromPoint()
      /home/pciet/go/src/github.com/pciet/wichess/wichessing/board_moves.go:214 +0x4a4
  github.com/pciet/wichess/wichessing.Board.AllNaiveMovesFor()
      /home/pciet/go/src/github.com/pciet/wichess/wichessing/board_moves.go:191 +0x1f1
  github.com/pciet/wichess/wichessing.Board.Check()
      /home/pciet/go/src/github.com/pciet/wichess/wichessing/board_check.go:39 +0xc4
  github.com/pciet/wichess/wichessing.Board.Checkmate()
      /home/pciet/go/src/github.com/pciet/wichess/wichessing/board_check.go:10 +0xb2
  main.listeningToGame.func2()
      /home/pciet/go/src/github.com/pciet/wichess/game_listening.go:114 +0x31b

Goroutine 49 (running) created at:
  net/http.(*Server).Serve()
      /usr/local/go/src/net/http/server.go:2720 +0x37c
  net/http.(*Server).ListenAndServe()
      /usr/local/go/src/net/http/server.go:2636 +0xc7
  net/http.ListenAndServe()
      /usr/local/go/src/net/http/server.go:2882 +0xfe
  main.main()
      /home/pciet/go/src/github.com/pciet/wichess/main.go:58 +0x5e6

Goroutine 125 (running) created at:
  main.listeningToGame()
      /home/pciet/go/src/github.com/pciet/wichess/game_listening.go:100 +0x7ec
  main.moveNotificationWebsocketHandler()
      /home/pciet/go/src/github.com/pciet/wichess/web_moven.go:74 +0x5e1
  net/http.HandlerFunc.ServeHTTP()
      /usr/local/go/src/net/http/server.go:1918 +0x51
  net/http.(*ServeMux).ServeHTTP()
      /usr/local/go/src/net/http/server.go:2254 +0xa2
  net/http.serverHandler.ServeHTTP()
      /usr/local/go/src/net/http/server.go:2619 +0xbc
  net/http.(*conn).serve()
      /usr/local/go/src/net/http/server.go:1801 +0x83b

path.go:
18		for path, _ := range paths {
19			availablepath := AbsPath{
20				Points: make([]AbsPoint, 0, len(*path)),
21			}
...
82 type AbsPath struct {
83 	Points    []AbsPoint
84 	Truncated bool
85 }

board_moves.go:
394		for path, _ := range unfilteredpaths {
395			if path.Truncated && the.MustEnd {
396				continue
397			}

The race is that an AbsPath was written into memory, then Truncated is read.

In this case the writing goroutine is one of those listening goroutines - it has put together a representation of all moves possible for the board state because it is determining if a checkmate state exists and would have returned if so (the goroutine received a notification that a move was made, or has just started and is checking initial conditions).

The reading goroutine is a player requesting what all moves are for the current game state, likely because they just moved and need the next move set.

There is no shared memory here besides the base representation of the moves for each piece, which is used to create new AbsPaths and is not referenced outside of TruncatedAbsPathsForKind. A Board instance is created from each database read and operated on in the reading goroutine - there are more database reads than necessary and more garbage memory created than necessary, but I'm getting it to work first before sharing memory via caching. The database interactions are mutex locked at the game level.

If the code triggers a race condition warning, then there's a race condition in the code and the program is not valid. Can you explain why do you believe that the reported race is not actually a race? Are you suggesting there's a bug in the race detector?

I've asked in the golang-nuts thread for a review of the TruncatedAbsPathsForKind function as it seems like a common source of the data races. But since these paths are created only for the calling goroutine I don't understand how different goroutines race on them. The crash indicates a pointer problem, which I've thought could be incorrect garbage collection or some sort of memory clobbering that tricks the race detector. Most likely I'm doing something wrong of course.

Nevertheless, Go tries not to crash just because there's a data race. (Go doesn't makes as many promises as Java in the face of a data race, but it does try not to crash.)

The server does not crash but does indicate a data race if I simulate four games or less (on a four core processor) but does crash within 30 seconds if the number of games increases. My test client is making moves and requesting available moves as fast as possible (~5 milliseconds per request) unsynchronized except for the interlock of playing the game.

[pciet]

Crash on go1.9.2 darwin/amd64 testing localhost (macOS 10.12.6) with client count 40:

http: panic serving [::1]:62998: runtime error: invalid memory address or nil pointer dereference
goroutine 771 [running]:
net/http.(*conn).serve.func1(0xc420088f00)
	/usr/local/go/src/net/http/server.go:1697 +0xd0
panic(0x13bbe60, 0x164a3e0)
	/usr/local/go/src/runtime/panic.go:491 +0x283
github.com/pciet/wichess/wichessing.Board.ActualPaths(0xc4203b8de0, 0x0, 0xc4203b8e10, 0x1, 0xc4203b8e40, 0x2, 0xc4203b8e70, 0x3, 0xc4203b8ea0, 0x4, ...)
	/Users/aftermatt/Code/src/github.com/pciet/wichess/wichessing/board_moves.go:402 +0x209
github.com/pciet/wichess/wichessing.Board.MovesFromPoint(0xc4203b8de0, 0x0, 0xc4203b8e10, 0x1, 0xc4203b8e40, 0x2, 0xc4203b8e70, 0x3, 0xc4203b8ea0, 0x4, ...)
	/Users/aftermatt/Code/src/github.com/pciet/wichess/wichessing/board_moves.go:215 +0x708
github.com/pciet/wichess/wichessing.Board.AllNaiveMovesFor(0xc4203b8de0, 0x0, 0xc4203b8e10, 0x1, 0xc4203b8e40, 0x2, 0xc4203b8e70, 0x3, 0xc4203b8ea0, 0x4, ...)
	/Users/aftermatt/Code/src/github.com/pciet/wichess/wichessing/board_moves.go:191 +0x18c
github.com/pciet/wichess/wichessing.Board.Check(0xc4203b8de0, 0x0, 0xc4203b8e10, 0x1, 0xc4203b8e40, 0x2, 0xc4203b8e70, 0x3, 0xc4203b8ea0, 0x4, ...)
	/Users/aftermatt/Code/src/github.com/pciet/wichess/wichessing/board_check.go:39 +0xb6
github.com/pciet/wichess/wichessing.Board.Moves(0xc420253000, 0x0, 0xc420253038, 0x1, 0xc420253070, 0x2, 0xc4202530a8, 0x3, 0xc4202530e0, 0x4, ...)
	/Users/aftermatt/Code/src/github.com/pciet/wichess/wichessing/board_moves.go:50 +0x4f8
main.game.moves(0x7, 0x20, 0x1, 0xc420534cf0, 0x8, 0x0, 0x30b42b90, 0xed1a28276, 0xc42010a420, 0x3c75eac3, ...)
	/Users/aftermatt/Code/src/github.com/pciet/wichess/game_moving.go:266 +0x719
main.movesHandler(0x161d4a0, 0xc420165420, 0xc420150e00)
	/Users/aftermatt/Code/src/github.com/pciet/wichess/web_moves.go:55 +0x31c
net/http.HandlerFunc.ServeHTTP(0x1448810, 0x161d4a0, 0xc420165420, 0xc420150e00)
	/usr/local/go/src/net/http/server.go:1918 +0x44
net/http.(*ServeMux).ServeHTTP(0x16580c0, 0x161d4a0, 0xc420165420, 0xc420150e00)
	/usr/local/go/src/net/http/server.go:2254 +0x130
net/http.serverHandler.ServeHTTP(0xc4200751e0, 0x161d4a0, 0xc420165420, 0xc420150e00)
	/usr/local/go/src/net/http/server.go:2619 +0xb4
net/http.(*conn).serve(0xc420088f00, 0x161dae0, 0xc42007d600)
	/usr/local/go/src/net/http/server.go:1801 +0x71d
created by net/http.(*Server).Serve
	/usr/local/go/src/net/http/server.go:2720 +0x288
runtime: nelems=256 nalloc=9 previous allocCount=8 nfreed=65535
fatal error: sweep increased allocation count

Full trace: https://gist.github.com/pciet/37e676ab8cbeac40bf3c4b0daed7dca4

No crash for two games (four clients).

Crashes on three games (six clients): https://gist.github.com/pciet/8c2b29c07fd548254b1bc3b02984a0ba

Crashes on four games (eight clients):

runtime: nelems=256 nalloc=126 previous allocCount=125 nfreed=65535
fatal error: sweep increased allocation count

runtime stack:
runtime.throw(0x143954b, 0x20)
	/usr/local/go/src/runtime/panic.go:605 +0x95 fp=0x700008a95cf0 sp=0x700008a95cd0 pc=0x102be55
runtime.(*mspan).sweep(0x1a09690, 0x1a09601, 0x0)
	/usr/local/go/src/runtime/mgcsweep.go:298 +0x911 fp=0x700008a95dc0 sp=0x700008a95cf0 pc=0x1021551
runtime.(*mcentral).cacheSpan(0x1661a90, 0xc420022560)
	/usr/local/go/src/runtime/mcentral.go:58 +0x2cd fp=0x700008a95e08 sp=0x700008a95dc0 pc=0x101671d
runtime.(*mcache).refill(0x18766c8, 0xc4203ed906, 0xc4204399f0)
	/usr/local/go/src/runtime/mcache.go:123 +0xa4 fp=0x700008a95e38 sp=0x700008a95e08 pc=0x10162e4
runtime.(*mcache).nextFree.func1()
	/usr/local/go/src/runtime/malloc.go:557 +0x32 fp=0x700008a95e60 sp=0x700008a95e38 pc=0x1053592
runtime.systemstack(0xc4202ca7b8)
	/usr/local/go/src/runtime/asm_amd64.s:344 +0x79 fp=0x700008a95e68 sp=0x700008a95e60 pc=0x10563a9
runtime.mstart()
	/usr/local/go/src/runtime/proc.go:1135 fp=0x700008a95e70 sp=0x700008a95e68 pc=0x1030650

goroutine 187 [running]:
runtime.systemstack_switch()
	/usr/local/go/src/runtime/asm_amd64.s:298 fp=0xc4205e3118 sp=0xc4205e3110 pc=0x1056320
runtime.(*mcache).nextFree(0x18766c8, 0x6, 0xc4205e3200, 0x122c0b0, 0x1617b20)
	/usr/local/go/src/runtime/malloc.go:556 +0xa9 fp=0xc4205e3170 sp=0xc4205e3118 pc=0x1011fd9
runtime.mallocgc(0x20, 0x13a8580, 0x1, 0x0)
	/usr/local/go/src/runtime/malloc.go:711 +0x6f0 fp=0xc4205e3218 sp=0xc4205e3170 pc=0x10128a0
runtime.newobject(0x13a8580, 0xc420576840)
	/usr/local/go/src/runtime/malloc.go:840 +0x38 fp=0xc4205e3248 sp=0xc4205e3218 pc=0x1012be8
github.com/lib/pq.(*conn).recv1(0xc42009ac80, 0xc4205e3331, 0xc420576840)
	/Users/aftermatt/Code/src/github.com/lib/pq/conn.go:793 +0x31 fp=0xc4205e3278 sp=0xc4205e3248 pc=0x122c3c1
github.com/lib/pq.(*conn).prepareTo(0xc42009ac80, 0xc4200ba080, 0x73, 0x0, 0x0, 0x0, 0x0, 0x1398900)
	/Users/aftermatt/Code/src/github.com/lib/pq/conn.go:566 +0x4bf fp=0xc4205e33e0 sp=0xc4205e3278 pc=0x122ad0f
github.com/lib/pq.(*conn).Exec(0xc42009ac80, 0xc4200ba080, 0x73, 0xc4201d40e0, 0x7, 0x7, 0x0, 0x0, 0x0, 0x0)
	/Users/aftermatt/Code/src/github.com/lib/pq/conn.go:660 +0x13b fp=0xc4205e3438 sp=0xc4205e33e0 pc=0x122babb
database/sql.ctxDriverExec(0x161db20, 0xc42007a1d8, 0x1a09220, 0xc42009ac80, 0xc4200ba080, 0x73, 0xc420270120, 0x7, 0x7, 0x102a729, ...)
	/usr/local/go/src/database/sql/ctxutil.go:43 +0x19c fp=0xc4205e34c0 sp=0xc4205e3438 pc=0x10f200c
database/sql.(*DB).execDC.func2()
	/usr/local/go/src/database/sql/sql.go:1253 +0x99 fp=0xc4205e3558 sp=0xc4205e34c0 pc=0x1100c39
database/sql.withLock(0x161b1e0, 0xc420134070, 0xc4205e36a8)
	/usr/local/go/src/database/sql/sql.go:2867 +0x65 fp=0xc4205e3580 sp=0xc4205e3558 pc=0x11004f5
database/sql.(*DB).execDC(0xc420089400, 0x161db20, 0xc42007a1d8, 0xc420134070, 0xc4205e3788, 0xc4200ba080, 0x73, 0xc420138580, 0x7, 0x8, ...)
	/usr/local/go/src/database/sql/sql.go:1252 +0x4e4 fp=0xc4205e3718 sp=0xc4205e3580 pc=0x10f83e4
database/sql.(*DB).exec(0xc420089400, 0x161db20, 0xc42007a1d8, 0xc4200ba080, 0x73, 0xc420138580, 0x7, 0x8, 0x1, 0xc4200ba080, ...)
	/usr/local/go/src/database/sql/sql.go:1238 +0x13e fp=0xc4205e37a8 sp=0xc4205e3718 pc=0x10f7eae
database/sql.(*DB).ExecContext(0xc420089400, 0x161db20, 0xc42007a1d8, 0xc4200ba080, 0x73, 0xc420138580, 0x7, 0x8, 0xc4200ba080, 0xc420256090, ...)
	/usr/local/go/src/database/sql/sql.go:1216 +0xe1 fp=0xc4205e3848 sp=0xc4205e37a8 pc=0x10f7ab1
database/sql.(*DB).Exec(0xc420089400, 0xc4200ba080, 0x73, 0xc420138580, 0x7, 0x8, 0x1, 0xc4205e3988, 0x14, 0x0)
	/usr/local/go/src/database/sql/sql.go:1230 +0x85 fp=0xc4205e38b8 sp=0xc4205e3848 pc=0x10f7d25
main.DB.updateGame(0xc420089400, 0x3, 0xc4205bd3b0, 0xc420285658, 0x7, 0x35, 0x25, 0x0)
	/Users/aftermatt/Code/src/github.com/pciet/wichess/game.go:203 +0xd2f fp=0xc4205e3c20 sp=0xc4205e38b8 pc=0x133b27f
main.game.move(0x3, 0x24, 0x1, 0xc420285658, 0x7, 0x0, 0x38500bd0, 0xed1a2866b, 0xc42010bc80, 0x13730fdca, ...)
	/Users/aftermatt/Code/src/github.com/pciet/wichess/game_moving.go:88 +0x952 fp=0xc4205e5c80 sp=0xc4205e3c20 pc=0x1341782
main.moveRequestHandler(0x161d4a0, 0xc42015a380, 0xc42012ca00)
	/Users/aftermatt/Code/src/github.com/pciet/wichess/web_move.go:104 +0xbf5 fp=0xc4205e7cb0 sp=0xc4205e5c80 pc=0x13501b5
net/http.HandlerFunc.ServeHTTP(0x1448808, 0x161d4a0, 0xc42015a380, 0xc42012ca00)
	/usr/local/go/src/net/http/server.go:1918 +0x44 fp=0xc4205e7cd8 sp=0xc4205e7cb0 pc=0x12bf724
net/http.(*ServeMux).ServeHTTP(0x16580c0, 0x161d4a0, 0xc42015a380, 0xc42012ca00)
	/usr/local/go/src/net/http/server.go:2254 +0x130 fp=0xc4205e7d18 sp=0xc4205e7cd8 pc=0x12c0ee0
net/http.serverHandler.ServeHTTP(0xc420075110, 0x161d4a0, 0xc42015a380, 0xc42012ca00)
	/usr/local/go/src/net/http/server.go:2619 +0xb4 fp=0xc4205e7d48 sp=0xc4205e7d18 pc=0x12c2454
net/http.(*conn).serve(0xc4201d20a0, 0x161dae0, 0xc420058980)
	/usr/local/go/src/net/http/server.go:1801 +0x71d fp=0xc4205e7fc8 sp=0xc4205e7d48 pc=0x12be5fd
runtime.goexit()
	/usr/local/go/src/runtime/asm_amd64.s:2337 +0x1 fp=0xc4205e7fd0 sp=0xc4205e7fc8 pc=0x1058f11
created by net/http.(*Server).Serve
	/usr/local/go/src/net/http/server.go:2720 +0x288

Full trace: https://gist.github.com/pciet/8189ffbb66bc46025186ceebccf2f6d6

This is a two core machine.

[ianlancetaylor]

The race detector has very very few false positives. It is telling you that there is no synchronization between the code that creates the AbsPath value and the code that reads the Truncated field from it. You should look carefully at your locking.

[RLH]

The GC keeps count of how many objects have been allocated since the last GC and how many were live at the last GC. This puts an upper bound on the number of live objects the GC expects to find live at the next GC. When the GC sweeps the span it checks to see if this upper bound was exceeded. In the past we have seen this check triggered by races resulting in pointers being hidden from the GC. We have also seen it when there is unsafe casting of pointers to uintptrs that the GC does not recognize as valid roots. And, of course, due to bugs in the GC. The first step is to get a minimized race free reproducer that doesn't use unsafe constructs.

…

On Sun, Nov 19, 2017 at 12:24 AM, Ian Lance Taylor ***@***.*** > wrote: The race detector has very very few false positives. It is telling you that there is no synchronization between the code that creates the AbsPath value and the code that reads the Truncated field from it. You should look carefully at your locking. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#22781 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA7WnyIGablKVqaG1gkdtGQieWYjil7bks5s37uCgaJpZM4QiKdE> .

[pciet]

Here's a simplified program that removes the application code and triggers the race and crash readily. The package wichessing does not contain the keyword 'go' and is an exact duplicate of the code in the original repository - the computation should have no concurrency. The only shared memory are global variables that are read and not written. My understanding is that there are no unsafe constructs.

https://github.com/pciet/debuggo22781

package main

import (
	"github.com/pciet/debuggo22781/wichessing"
)

const clients = 6

func main() {
	for i := 0; i < clients; i++ {
		go simulateClient()
	}
	select {} // block forever
}

func simulateClient() {
	for {
		board := wichessing.Board{}
		for i := 0; i < 64; i++ {
			board[i] = wichessing.Point{
				AbsPoint: wichessing.AbsPoint{wichessing.FileFromIndex(uint8(i)), wichessing.RankFromIndex(uint8(i))},
			}
		}
		setChessStart(&board)
		_, _, _ = board.Moves(wichessing.White, wichessing.AbsPoint{0, 8}, wichessing.AbsPoint{0, 8})
	}
}

func setChessStart(b *wichessing.Board) {
	b[0].Piece = &wichessing.Piece{
		Kind:        wichessing.Rook,
		Orientation: wichessing.White,
		Base:        wichessing.Rook,
	}
	b[1].Piece = &wichessing.Piece{
		Kind:        wichessing.Knight,
		Orientation: wichessing.White,
		Base:        wichessing.Knight,
	}
	b[2].Piece = &wichessing.Piece{
		Kind:        wichessing.Bishop,
		Orientation: wichessing.White,
		Base:        wichessing.Bishop,
	}
	b[3].Piece = &wichessing.Piece{
		Kind:        wichessing.Queen,
		Orientation: wichessing.White,
		Base:        wichessing.Queen,
	}
	b[4].Piece = &wichessing.Piece{
		Kind:        wichessing.King,
		Orientation: wichessing.White,
		Base:        wichessing.King,
	}
	b[5].Piece = &wichessing.Piece{
		Kind:        wichessing.Bishop,
		Orientation: wichessing.White,
		Base:        wichessing.Bishop,
	}
	b[6].Piece = &wichessing.Piece{
		Kind:        wichessing.Knight,
		Orientation: wichessing.White,
		Base:        wichessing.Knight,
	}
	b[7].Piece = &wichessing.Piece{
		Kind:        wichessing.Rook,
		Orientation: wichessing.White,
		Base:        wichessing.Rook,
	}
	for i := 0; i < 8; i++ {
		b[8+i].Piece = &wichessing.Piece{
			Kind:        wichessing.Pawn,
			Orientation: wichessing.White,
			Base:        wichessing.Pawn,
		}
	}
	b[63].Piece = &wichessing.Piece{
		Kind:        wichessing.Rook,
		Orientation: wichessing.Black,
		Base:        wichessing.Rook,
	}
	b[62].Piece = &wichessing.Piece{
		Kind:        wichessing.Knight,
		Orientation: wichessing.Black,
		Base:        wichessing.Knight,
	}
	b[61].Piece = &wichessing.Piece{
		Kind:        wichessing.Bishop,
		Orientation: wichessing.Black,
		Base:        wichessing.Bishop,
	}
	b[60].Piece = &wichessing.Piece{
		Kind:        wichessing.King,
		Orientation: wichessing.Black,
		Base:        wichessing.King,
	}
	b[59].Piece = &wichessing.Piece{
		Kind:        wichessing.Queen,
		Orientation: wichessing.Black,
		Base:        wichessing.Queen,
	}
	b[58].Piece = &wichessing.Piece{
		Kind:        wichessing.Bishop,
		Orientation: wichessing.Black,
		Base:        wichessing.Bishop,
	}
	b[57].Piece = &wichessing.Piece{
		Kind:        wichessing.Knight,
		Orientation: wichessing.Black,
		Base:        wichessing.Knight,
	}
	b[56].Piece = &wichessing.Piece{
		Kind:        wichessing.Rook,
		Orientation: wichessing.Black,
		Base:        wichessing.Rook,
	}
	for i := 0; i < 8; i++ {
		b[63-16+i].Piece = &wichessing.Piece{
			Kind:        wichessing.Pawn,
			Orientation: wichessing.Black,
			Base:        wichessing.Pawn,
		}
	}
	for i := 0; i < 64; i++ {
		if b[i].Piece == nil {
			continue
		}
		p := b[i].Piece.SetKindFlags()
		b[i].Piece = &p
	}
}

The race and crash are easy to trigger on my machine, although the crash signature is different.

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x1 pc=0x10f9884]

Races and crash trace here: https://gist.github.com/pciet/9c2251324184adbe3fdd25e6ded9160b

[RLH]

The race should be fixed first to see if that resolves the issue. One bug at a time. The GC is concurrent and it is not hard to imagine that an application race could hide pointers from the GC.

…

On Sun, Nov 19, 2017 at 1:18 PM, Matt Juran ***@***.***> wrote: Here's a simplified program that removes the application code and triggers the race and crash readily. The package wichessing does not contain the keyword 'go' and is an exact duplicate of the code in the original repository - the computation should have no concurrency. The only shared memory are global variables that are read and not written. My understanding is that there are no unsafe constructs. https://github.com/pciet/debuggo22781 package main import ( "github.com/pciet/debuggo22781/wichessing" ) const clients = 6 func main() { for i := 0; i < clients; i++ { go simulateClient() } select {} // block forever } func simulateClient() { for { board := wichessing.Board{} for i := 0; i < 64; i++ { board[i] = wichessing.Point{ AbsPoint: wichessing.AbsPoint{wichessing.FileFromIndex(uint8(i)), wichessing.RankFromIndex(uint8(i))}, } } setChessStart(&board) _, _, _ = board.Moves(wichessing.White, wichessing.AbsPoint{0, 8}, wichessing.AbsPoint{0, 8}) } } func setChessStart(b *wichessing.Board) { b[0].Piece = &wichessing.Piece{ Kind: wichessing.Rook, Orientation: wichessing.White, Base: wichessing.Rook, } b[1].Piece = &wichessing.Piece{ Kind: wichessing.Knight, Orientation: wichessing.White, Base: wichessing.Knight, } b[2].Piece = &wichessing.Piece{ Kind: wichessing.Bishop, Orientation: wichessing.White, Base: wichessing.Bishop, } b[3].Piece = &wichessing.Piece{ Kind: wichessing.Queen, Orientation: wichessing.White, Base: wichessing.Queen, } b[4].Piece = &wichessing.Piece{ Kind: wichessing.King, Orientation: wichessing.White, Base: wichessing.King, } b[5].Piece = &wichessing.Piece{ Kind: wichessing.Bishop, Orientation: wichessing.White, Base: wichessing.Bishop, } b[6].Piece = &wichessing.Piece{ Kind: wichessing.Knight, Orientation: wichessing.White, Base: wichessing.Knight, } b[7].Piece = &wichessing.Piece{ Kind: wichessing.Rook, Orientation: wichessing.White, Base: wichessing.Rook, } for i := 0; i < 8; i++ { b[8+i].Piece = &wichessing.Piece{ Kind: wichessing.Pawn, Orientation: wichessing.White, Base: wichessing.Pawn, } } b[63].Piece = &wichessing.Piece{ Kind: wichessing.Rook, Orientation: wichessing.Black, Base: wichessing.Rook, } b[62].Piece = &wichessing.Piece{ Kind: wichessing.Knight, Orientation: wichessing.Black, Base: wichessing.Knight, } b[61].Piece = &wichessing.Piece{ Kind: wichessing.Bishop, Orientation: wichessing.Black, Base: wichessing.Bishop, } b[60].Piece = &wichessing.Piece{ Kind: wichessing.King, Orientation: wichessing.Black, Base: wichessing.King, } b[59].Piece = &wichessing.Piece{ Kind: wichessing.Queen, Orientation: wichessing.Black, Base: wichessing.Queen, } b[58].Piece = &wichessing.Piece{ Kind: wichessing.Bishop, Orientation: wichessing.Black, Base: wichessing.Bishop, } b[57].Piece = &wichessing.Piece{ Kind: wichessing.Knight, Orientation: wichessing.Black, Base: wichessing.Knight, } b[56].Piece = &wichessing.Piece{ Kind: wichessing.Rook, Orientation: wichessing.Black, Base: wichessing.Rook, } for i := 0; i < 8; i++ { b[63-16+i].Piece = &wichessing.Piece{ Kind: wichessing.Pawn, Orientation: wichessing.Black, Base: wichessing.Pawn, } } for i := 0; i < 64; i++ { if b[i].Piece == nil { continue } p := b[i].Piece.SetKindFlags() b[i].Piece = &p } } The race and crash are easy to trigger on my machine, although the crash signature is different. panic: runtime error: invalid memory address or nil pointer dereference [signal SIGSEGV: segmentation violation code=0x1 addr=0x1 pc=0x10f9884] Races and crash trace here: https://gist.github.com/pciet/ 9c2251324184adbe3fdd25e6ded9160b — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#22781 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA7Wn1XGYy7fUT0jTs-5hMBIgzOB_nEpks5s4HEIgaJpZM4QiKdE> .

[pciet]

I am making the claim that there is no fixable race in this program as each goroutine is independent by only reading from shared memory (no shared writes).

The shared reads are in the first two functions of wichessing/path.go where written memory is created independently for each calling goroutine (this is also where the race is indicated). The shared memory is read for the for loop computation and otherwise unused (len, addition, then an addition or subtraction). The written memory is not shared between goroutines.

[bradfitz]

Normally when somebody blames the race detector, we're all very skeptical (see @ianlancetaylor's comment earlier), and I admit (from looking at this a few minutes) that the race detector isn't super helpful here.

Notes: (all on Linux)

-- there is only one "go" statement in the code.
-- the shared memory is set up in inits in paths.go
-- no reported race with Go 1.8
-- races reported (with missing stacks) with Go1.9 and master.

The missing stacks are usually lost:

Previous write at 0x00c4200140a7 by goroutine 9:
  [failed to restore the stack]

When they're not lost, they point to:

Previous write at 0x00c420480a78 by goroutine 8:
  github.com/pciet/debuggo22781/wichessing.TruncatedAbsPathsForKind()
      /home/bradfitz/src/github.com/pciet/debuggo22781/wichessing/path.go:19 +0x3d2
  github.com/pciet/debuggo22781/wichessing.Board.MovesFromPoint()
      /home/bradfitz/src/github.com/pciet/debuggo22781/wichessing/board_moves.go:214 +0x4a4
  github.com/pciet/debuggo22781/wichessing.Board.Move()
      /home/bradfitz/src/github.com/pciet/debuggo22781/wichessing/board_move.go:126 +0x12b1
  github.com/pciet/debuggo22781/wichessing.Board.AfterMove()
      /home/bradfitz/src/github.com/pciet/debuggo22781/wichessing/board_move.go:10 +0x18a
  github.com/pciet/debuggo22781/wichessing.Board.Check()
      /home/bradfitz/src/github.com/pciet/debuggo22781/wichessing/board_check.go:47 +0x2ae
  github.com/pciet/debuggo22781/wichessing.Board.Checkmate()
      /home/bradfitz/src/github.com/pciet/debuggo22781/wichessing/board_check.go:10 +0xb2
  github.com/pciet/debuggo22781/wichessing.Board.Moves()
      /home/bradfitz/src/github.com/pciet/debuggo22781/wichessing/board_moves.go:12 +0xd3
  main.simulateClient()
      /home/bradfitz/src/github.com/pciet/debuggo22781/main.go:28 +0xc9

Where line 19 is:

// This function will truncate moves that leave the board and convert the defined relative points to actual board points but does \
not remove points with other pieces or other similar constraints.                                                                  
func TruncatedAbsPathsForKind(the Kind, from AbsPoint, with Orientation) AbsPathSetMap {
        absmap := make(AbsPathSetMap)
        for movetype, paths := range RelPathMapForKind(the) {
                availablepaths := make(AbsPathSet)
                for path, _ := range paths {
                        availablepath := AbsPath{   // <---- Line 19
                                Points: make([]AbsPoint, 0, len(*path)),
                        }

I highly suspect the problem is this line:

         availablepath.Points = append(availablepath.Points, AbsPoint{File: uint8(absfile), Rank: uint8(absrank)})

Appends on shared memory are always a red flag in concurrent code.

I'm busy now for a few hours, but I can test that theory later unless others beat me to it.

[bradfitz]

Appends on shared memory are always a red flag in concurrent code.

(although I'm not convinced that memory is shared yet, either.)

The append was just my next area for investigation.

[ALTree]

no reported race with Go 1.8

I also noticed this, so I started bisecting.

The first commit that triggers a race report on the reproducer above is ec091b6 (runtime: add mapassign_fast*).

It's possible that the race is real, but it's triggered by bad code generation and not by an error in the Go code itself. Next step: investigate changes in code generation introduced in the reproducer by that commit.