Bdtjtk

I am reading a book which teaches me how to write a simple cache like redis.

With a goal to implement a distribute hash, the project must have key migrate, which needs an iterator. And I think there may be some problems.

His book about iterating a map, but while the iteration, the hold of read lock not continuously. The reason is trying not to effect main cache process. I believe there must be a thread safety problem because the main cache thread is still writing to map. I wrote a demo, but not sure.

//book code

type inMemoryScanner struct {

    pair

    pair Chan *pair

    closeCh chan struct{}

}



func (c *inMemoryCache) NewScanner() Scanner {

    pairCh := make(chan *pair)

    closeCh := make(chan struct{})

    go func() {

        defer close(pairCh)

        c.mutex.RLock()

        //the c.c is book's map

        for k, v := range c.c {

            c.mutex.RUnlock()

            select {

            case <-closeCh:

                return

            case pairCh <- &pair{k, v}:

            }

            c.mutex.RLock()

        }

        c.mutex.RUnLock()

     }

     return &inMemoryScanner{pair{}, pairCh, closeCh}

}

//my demo

func main()  {

    testMap := make(map[string]string)

    mutex := sync.RWMutex{}

    for i := 0; i < 64; i ++ {

        mutex.Lock()

        testMap[uuid.New().String()] = uuid.New().String()

        mutex.Unlock()

        fmt.Println("Write")

    }

    go func() {

        for {

            mutex.Lock()

            testMap[uuid.New().String()] = uuid.New().String()

            time.Sleep(100 * time.Millisecond)

            mutex.Unlock()

            fmt.Println("Write")

        }

    } ()

    for k, v := range testMap {

        mutex.RLock()

        fmt.Println("k" + k + "v" + v)

        mutex.RUnlock()

        time.Sleep(100 * time.Millisecond)

    }

}

In my demo, the 'Write' and the map's result amount not equal! And I believe, In an reality project, the rebalance can't be once, there must be continuous background work, doesn't it?

You have a data race. Your results are undefined.

Simplifying your code so that it compiles and runs,

package main



import (

    "sync"

    "time"

)



func main() {

    testMap := make(map[string]string)

    mutex := sync.RWMutex{}

    for i := 0; i < 64; i++ {

        mutex.Lock()

        now := time.Now().String()

        testMap[now] = now

        mutex.Unlock()

    }

    go func() {

        for {

            mutex.Lock()

            now := time.Now().String()

            testMap[now] = now

            time.Sleep(100 * time.Millisecond)

            mutex.Unlock()

        }

    }()

    for k, v := range testMap {

        mutex.RLock()

        _, _ = k, v

        mutex.RUnlock()

        time.Sleep(100 * time.Millisecond)

    }

}

Output:

$ go run -race racer.go

==================

WARNING: DATA RACE

Read at 0x00c00008c060 by main goroutine:

  runtime.mapiternext()

      /home/peter/go/src/runtime/map.go:844 +0x0

  main.main()

      /home/peter/gopath/src/racer.go:26 +0x217



Previous write at 0x00c00008c060 by goroutine 5:

  runtime.mapassign_faststr()

      /home/peter/go/src/runtime/map_faststr.go:202 +0x0

  main.main.func1()

      /home/peter/gopath/src/racer.go:21 +0x9b



Goroutine 5 (running) created at:

  main.main()

      /home/peter/gopath/src/racer.go:17 +0x17b

==================

==================

WARNING: DATA RACE

Read at 0x00c0000a6638 by main goroutine:

  main.main()

      /home/peter/gopath/src/racer.go:26 +0x1d0



Previous write at 0x00c0000a6638 by goroutine 5:

  main.main.func1()

      /home/peter/gopath/src/racer.go:21 +0xb0



Goroutine 5 (running) created at:

  main.main()

      /home/peter/gopath/src/racer.go:17 +0x17b

==================

fatal error: concurrent map iteration and map write



goroutine 1 [running]:

runtime.throw(0x4b1eb7, 0x26)

    /home/peter/go/src/runtime/panic.go:617 +0x72 fp=0xc000059e48 sp=0xc000059e18 pc=0x44d722

runtime.mapiternext(0xc000059f28)

    /home/peter/go/src/runtime/map.go:851 +0x55e fp=0xc000059ed0 sp=0xc000059e48 pc=0x434c2e

main.main()

    /home/peter/gopath/src/racer.go:26 +0x218 fp=0xc000059f98 sp=0xc000059ed0 pc=0x48a1f8

runtime.main()

    /home/peter/go/src/runtime/proc.go:200 +0x20c fp=0xc000059fe0 sp=0xc000059f98 pc=0x44f06c

runtime.goexit()

    /home/peter/go/src/runtime/asm_amd64.s:1337 +0x1 fp=0xc000059fe8 sp=0xc000059fe0 pc=0x475751



goroutine 4 [sleep]:

runtime.goparkunlock(...)

    /home/peter/go/src/runtime/proc.go:307

time.Sleep(0x5f5e100)

    /home/peter/go/src/runtime/time.go:105 +0x159

main.main.func1(0xc00001c280, 0xc00008c060)

    /home/peter/gopath/src/racer.go:22 +0x3e

created by main.main

    /home/peter/gopath/src/racer.go:17 +0x17c

exit status 2

$ 

You are not locking the map reads,

for k, v := range testMap {

    mutex.RLock()

    _, _ = k, v

    mutex.RUnlock()

    time.Sleep(100 * time.Millisecond)

}

for k, v := range testMap { ... } reads the map. k, v are local variables.

You need to lock the map reads,

mutex.RLock()

for k, v := range testMap {

    _, _ = k, v

    time.Sleep(100 * time.Millisecond)

}

mutex.RUnlock()

Go: Data Race Detector

The Go Blog: Introducing the Go Race Detector

GopherCon 2016: Keith Randall - Inside the Map Implementation