unexpected panic when struct fields comparison with sync.WaitGroup misuse

[twz915]

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

$ go version
go version go1.13.5 darwin/amd64

Does this issue reproduce with the latest release?

yes

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

go env Output

$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/tu/Library/Caches/go-build"
GOENV="/Users/tu/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/tu/.go"
GOPROXY="https://goproxy.cn,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/tu/workspace/footprint/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/v2/xjx1mlfd43x39dxyyfjn64kr0000gp/T/go-build415442482=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

package main

import "sync"

type Payload struct {
	A string
	B string
}

func main() {
	for i := 0; i < 20; i++ {
		go func() {
			for {
				payload := &Payload{}

				var wg sync.WaitGroup
				wg.Add(1)
				go func() {
					defer wg.Done()
					payload.A += "1"
				}()
				go func() {
					payload.B += "1"
				}()
				wg.Wait()

				if payload.A != payload.B { // Crash at here.
				}
			}
		}()
	}
	<-make(chan bool)
}

What did you expect to see?

no panic

What did you see instead?

panic

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

goroutine 23 [running]:
main.main.func1()
	/private/var/folders/v2/xjx1mlfd43x39dxyyfjn64kr0000gp/T/CodeRunner/Go/Untitled 2.go:27 +0xf0
created by main.main
	/private/var/folders/v2/xjx1mlfd43x39dxyyfjn64kr0000gp/T/CodeRunner/Go/Untitled 2.go:12 +0x3e

I googled these links

1. https://stackoverflow.com/questions/29496605/is-it-thread-safe-to-access-different-members-of-struct-in-go/29497244#29497244
2. https://stackoverflow.com/questions/54457213/how-to-assign-the-values-to-struct-while-go-routines-are-running

Why it crashed when string comparison?
Consider the following example

package main

type Payload struct {
	A string
	B string
}

func main() {
	payload := &Payload{}
	
	for i:=0;i<100;i++{
		go func(){
			for {
				payload.A += "1"
				if payload.A != payload.B { // no crash
				}
			}
		}()
		
		go func(){
			for {
				payload.B += "1"

				if payload.A != payload.B { // no crash
				}
			}
		}()
	}
	
	<- make(chan int)
}

[kortschak]

There is a race in your code; the second small goroutine does not refer to the wait group at all. If you add that in, and adjust the wg.Add call to take that into account, the code works as expected.

[ianlancetaylor]

I agree: this has nothing to do with sync.WaitGroup or with accessing different member of structs. You have a race on a string value: one goroutine is reading it while another goroutine is simultaneously writing to it. That can lead to a crash.

I'm going to close this since I don't think there is anything to change in the Go tools or libraries. You may want to look into using the race detector (https://golang.org/doc/articles/race_detector.html).

[twz915]

@kortschak @ianlancetaylor why the last demo without crash?

[ianlancetaylor]

Races are unpredictable. There may be a reason why that program doesn't crash, but I don't know what it is, and I don't think it matters. Programs with race conditions are never safe in Go. Don't write them. Use the race detector.