From 946fa21dc760ba38acd0b49593f604b98c4ec6e3 Mon Sep 17 00:00:00 2001 From: Jae Kwon Date: Sun, 28 Dec 2014 17:10:03 -0800 Subject: [PATCH] fix race conditions --- Makefile | 3 +++ binary/reflect.go | 31 +++++++++++++++++++------------ consensus/reactor.go | 4 ++++ 3 files changed, 26 insertions(+), 12 deletions(-) diff --git a/Makefile b/Makefile index 294967564..ebb1d9dbe 100644 --- a/Makefile +++ b/Makefile @@ -3,6 +3,9 @@ all: build build: go build -o tendermint github.com/tendermint/tendermint/cmd +build_race: + go build -race -o tendermint github.com/tendermint/tendermint/cmd + test: go test github.com/tendermint/tendermint/... diff --git a/binary/reflect.go b/binary/reflect.go index fda0fc49f..0c1a75324 100644 --- a/binary/reflect.go +++ b/binary/reflect.go @@ -5,6 +5,7 @@ import ( "fmt" "io" "reflect" + "sync" ) type TypeInfo struct { @@ -27,8 +28,23 @@ type HasTypeByte interface { TypeByte() byte } +// NOTE: do not access typeInfos directly, but call GetTypeInfo() +var typeInfosMtx sync.Mutex var typeInfos = map[reflect.Type]*TypeInfo{} +func GetTypeInfo(rt reflect.Type) *TypeInfo { + typeInfosMtx.Lock() + defer typeInfosMtx.Unlock() + info := typeInfos[rt] + if info == nil { + info = &TypeInfo{Type: rt} + RegisterType(info) + } + return info +} + +// Registers and possibly modifies the TypeInfo. +// NOTE: not goroutine safe, so only call upon program init. func RegisterType(info *TypeInfo) *TypeInfo { // Register the type info @@ -74,10 +90,7 @@ func readReflect(rv reflect.Value, rt reflect.Type, r io.Reader, n *int64, err * } // Get typeInfo - typeInfo := typeInfos[rt] - if typeInfo == nil { - typeInfo = RegisterType(&TypeInfo{Type: rt}) - } + typeInfo := GetTypeInfo(rt) // Custom decoder if typeInfo.Decoder != nil { @@ -178,10 +191,7 @@ func readReflect(rv reflect.Value, rt reflect.Type, r io.Reader, n *int64, err * func writeReflect(rv reflect.Value, rt reflect.Type, w io.Writer, n *int64, err *error) { // Get typeInfo - typeInfo := typeInfos[rt] - if typeInfo == nil { - typeInfo = RegisterType(&TypeInfo{Type: rt}) - } + typeInfo := GetTypeInfo(rt) // Custom encoder, say for an interface type rt. if typeInfo.Encoder != nil { @@ -193,11 +203,8 @@ func writeReflect(rv reflect.Value, rt reflect.Type, w io.Writer, n *int64, err if rt.Kind() == reflect.Interface { rv = rv.Elem() rt = rv.Type() - typeInfo = typeInfos[rt] // If interface type, get typeInfo of underlying type. - if typeInfo == nil { - typeInfo = RegisterType(&TypeInfo{Type: rt}) - } + typeInfo = GetTypeInfo(rt) } // Dereference pointer diff --git a/consensus/reactor.go b/consensus/reactor.go index 1b20676eb..07365e5ed 100644 --- a/consensus/reactor.go +++ b/consensus/reactor.go @@ -323,7 +323,11 @@ OUTER_LOOP: trySendVote := func(voteSet *VoteSet, peerVoteSet BitArray) (sent bool) { // TODO: give priority to our vote. + // peerVoteSet BitArray is being accessed concurrently with + // writes from Receive() routines. We must lock like so here: + ps.mtx.Lock() index, ok := voteSet.BitArray().Sub(peerVoteSet).PickRandom() + ps.mtx.Unlock() if ok { vote := voteSet.GetByIndex(index) // NOTE: vote may be a commit.