Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

runtime: fatal ERROR: concurrent map read and map write #26010

Closed
thelark opened this issue Jun 22, 2018 · 6 comments

Comments

Projects
None yet
5 participants
@thelark
Copy link

commented Jun 22, 2018

Please answer these questions before submitting your issue. Thanks!

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

go version go1.10 windows/amd64

Does this issue reproduce with the latest release?

yes

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

set GOARCH=amd64
set GOBIN=
set GOCACHE=C:\Users\Administrator\AppData\Local\go-build
set GOEXE=.exe
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOOS=windows
set GOPATH=D:\work
set GORACE=
set GOROOT=C:\Go
set GOTMPDIR=
set GOTOOLDIR=C:\Go\pkg\tool\windows_amd64
set GCCGO=gccgo
set CC=gcc
set CXX=g++
set CGO_ENABLED=1
set CGO_CFLAGS=-g -O2
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-g -O2
set CGO_FFLAGS=-g -O2
set CGO_LDFLAGS=-g -O2
set PKG_CONFIG=pkg-config
set GOGCCFLAGS=-m64 -mthreads -fmessage-length=0 -fdebug-prefix-map=C:\Users\ADMINI~1\AppData\Local\Temp\go-build514560930=/tmp/go-build -gno-record-gcc-switches

What did you do?

I create sync.Map in a file and pass it in another file when parameters are passed in.

What did you expect to see?

Sync.Map is thread safe, but it gave me the wrong report.

What did you see instead?

fatal error: concurrent map read and map write

@davecheney

This comment has been minimized.

Copy link
Contributor

commented Jun 22, 2018

Please provide a runnable code sample to demonstrate this issue,

Please check that your code sample will pass when built with the -race flag. See https://blog.golang.org/race-detector .

@fraenkel

This comment has been minimized.

Copy link
Contributor

commented Jun 22, 2018

Just so you know, that error message only comes from the runtime map. sync.map does not have that error message.

@ianlancetaylor ianlancetaylor changed the title sync.Map fatal ERROR: concurrent map read and map write runtime: fatal ERROR: concurrent map read and map write Jun 22, 2018

@ianlancetaylor ianlancetaylor added this to the Unplanned milestone Jun 22, 2018

@josharian

This comment has been minimized.

Copy link
Contributor

commented Jun 22, 2018

@fraenkel sync.Map uses regular maps under the hood

@fraenkel

This comment has been minimized.

Copy link
Contributor

commented Jun 22, 2018

@josharian Forgot about that. My guess without seeing the testcase is that its the common issue where the map is copied across go routines.

@thelark

This comment has been minimized.

Copy link
Author

commented Jun 25, 2018

package main

import (
	"sync"
	"net"
	"fmt"
	"time"
	"strings"
	"math/rand"
	"encoding/hex"
)

var msg = []string{
	"2461707550350150412506182253068502114303167c000000ffffb9ff000000000000000001cc002639131c0e",
	"*HQ,000,00,00000,0000#",
	"1212231273862178963863217984abc12863879a8c6a6c76a98c6ac9a6c798a6c76a",
}

func init() {
	rand.Seed(time.Now().UnixNano())
}
func main() {
	li, err := net.Listen("tcp", ":2277")
	if err != nil {
		panic(err)
	}
	defer li.Close()

	go func() {
		data1, _ := hex.DecodeString(msg[0])
		data2, _ := hex.DecodeString(msg[2])
		var datas = [][]byte{
			[]byte(msg[1]),
			data1,
			data2,
		}

		for {
			for i := 0; i < len(datas); i++ {
				go func(i int) {
					li, _ := net.Dial("tcp", ":2277")
					li.Write(datas[i])
					li.Close()
				}(i)
			}
			time.Sleep(time.Second)
		}
	}()

	for {
		conn, err := li.Accept()
		if err != nil {
			fmt.Println(err)
			time.Sleep(5 * time.Second)
			continue
		}
		go recvConnMsg(conn)
	}
}
func recvConnMsg(conn net.Conn) {
	var (
		aMap sync.Map
		bMap sync.Map
	)
	buf := make([]byte, 4096)
	defer conn.Close()
	aMap.Store(conn, "")
	go InitData(conn, aMap, bMap)
	for {
		n, err := conn.Read(buf)
		if err != nil {
			fmt.Println("conn closed")
			return
		}
		Hex := fmt.Sprintf("%x", buf[0:n])
		if strings.Contains(Hex, "1212") {
			connMsgVal, _ := aMap.Load(conn)
			aMap.Store(conn, connMsgVal.(string)+Hex)
		} else {
			if strings.HasPrefix(string(buf[0:n]), "*") {
				Heartbeat(conn, bMap, string(buf[0:n]))
			} else {
				Basic(conn, bMap, Hex)
			}
		}
	}
}

func Heartbeat(conn net.Conn, connMap sync.Map, connMsgValStr string) {
	startIndex := strings.Index(connMsgValStr, "*")
	endIndex := strings.Index(connMsgValStr, "#")
	if endIndex < startIndex {
		endIndex = strings.Index(connMsgValStr[endIndex+1:], "#")
	}
	connMap.Store(conn, rand.Intn(100))
	fmt.Println(startIndex, endIndex)
}
func Basic(conn net.Conn, connMap sync.Map, connMsgValStr string) {
	startIndex := strings.Index(connMsgValStr, "24")
	if startIndex < 0 {
		return
	}
	connMap.Store(conn, rand.Intn(100))
	fmt.Println(startIndex)
}
func InitData(conn net.Conn, aMap, bMap sync.Map) {
	for {
		aData, ok := aMap.Load(conn)
		if !ok || aData == "" {
			continue
		}
		connMsgValStr := aData.(string)
		currentIndex := strings.Index(connMsgValStr, "1212") 
		if currentIndex == -1 {
			continue
		}
		AnalyzePackage(conn, bMap)
		aMap.Store(conn, string(connMsgValStr[currentIndex:]))
		time.Sleep(time.Second)
	}
}
func AnalyzePackage(conn net.Conn, bMap sync.Map) {
	key, ok := bMap.Load(conn)
	if ok {
		fmt.Println(key)
	} else {
		bMap.Store(key, rand.Intn(100))
	}
}

some times, they will be fatal error: concurrent map read and map write. For example, when a request occurs at the same time, they will be fatal error: concurrent map read and map write.

@davecheney

This comment has been minimized.

Copy link
Contributor

commented Jun 25, 2018

func Heartbeat(conn net.Conn, connMap sync.Map, connMsgValStr string) {
	startIndex := strings.Index(connMsgValStr, "*")
	endIndex := strings.Index(connMsgValStr, "#")
	if endIndex < startIndex {
		endIndex = strings.Index(connMsgValStr[endIndex+1:], "#")
	}
	connMap.Store(conn, rand.Intn(100))
	fmt.Println(startIndex, endIndex)
}

You must pass a pointer to your sync.Map, connMap *sync.Map.

I am going to close this as there is no issue and is handled by go vet

Daves-MBP(~/src) % go vet vm.go
# command-line-arguments
./vm.go:64: call of InitData copies lock value: sync.Map contains sync.Mutex
./vm.go:64: call of InitData copies lock value: sync.Map contains sync.Mutex
./vm.go:77: call of Heartbeat copies lock value: sync.Map contains sync.Mutex
./vm.go:79: call of Basic copies lock value: sync.Map contains sync.Mutex
./vm.go:85: Heartbeat passes lock by value: sync.Map contains sync.Mutex
./vm.go:94: Basic passes lock by value: sync.Map contains sync.Mutex
./vm.go:102: InitData passes lock by value: sync.Map contains sync.Mutex
./vm.go:113: call of AnalyzePackage copies lock value: sync.Map contains sync.Mutex
./vm.go:118: AnalyzePackage passes lock by value: sync.Map contains sync.Mutex

@davecheney davecheney closed this Jun 25, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.