Golang data race cause by consurrent map read-CodePudding

I have a server to handle events, this server has a mutex lock and a events table(map structure). When the server receives a new event, it will acquire lock to prevent data race, store this event in the events table, and start a goroutine to monitor this event has done. If I run the program with -race flag, it will output data race.

package main

import (
    "sync"
    "time"
)

type event struct {
    done chan bool
}

type server struct {
    mu     sync.Mutex
    events map[int]*event
}

func main() {
    s := server{}
    s.events = make(map[int]*event)

    for i := 0; i < 10; i   {
        go func(i int) {
            s.mu.Lock()
            s.events[i] = &event{}
            s.events[i].done = make(chan bool)
            s.mu.Unlock()
            go func() {
                time.Sleep(1 * time.Millisecond)
                <-s.events[i].done
                // server do something.
            }()
        }(i)
    }

    time.Sleep(1 * time.Second)

    for i := 0; i < 10; i   {
        // event happen.
        s.events[i].done <- true
    }
}

Output

==================
WARNING: DATA RACE
Read at 0x00c00010dd10 by goroutine 14:
  runtime.mapaccess1_fast64()
      c:/go/src/runtime/map_fast64.go:12  0x0    
  main.main.func1.1()
      C:/SimpleAsyncBFT/race/main.go:29  0x7c    

Previous write at 0x00c00010dd10 by goroutine 15:
  runtime.mapassign_fast64()
      c:/go/src/runtime/map_fast64.go:92  0x0    
  main.main.func1()
      C:/SimpleAsyncBFT/race/main.go:24  0xbe    

Goroutine 14 (running) created at:
  main.main.func1()
      C:/SimpleAsyncBFT/race/main.go:27  0x1c6

Goroutine 15 (finished) created at:
  main.main()
      C:/SimpleAsyncBFT/race/main.go:22  0xed

I know adding lock in the monitor goroutine will solve this problem, but will cause deadlock! The done channel is just used to notify the server that the event has been done. If channel was not suitable for this condition, how to achieve this?

CodePudding user response：

As per the comments your code attempts to read and write to a map simultaneously and, as per the go 1.6 release notes:

if one goroutine is writing to a map, no other goroutine should be reading or writing the map concurrently

Looking at your code there appears to be no need for this. You can create the channels in advance; after they are created you are only reading from the map so there is no issue:

package main

import (
    "sync"
    "time"
)

type event struct {
    done chan bool
}

type server struct {
    mu     sync.Mutex
    events map[int]*event
}

func main() {
    s := server{}
    s.events = make(map[int]*event)

    for i := 0; i < 10; i   {
        s.events[i] = &event{}
        s.events[i].done = make(chan bool)
    }

    for i := 0; i < 10; i   {
        go func(i int) {
            time.Sleep(1 * time.Millisecond)
            <-s.events[i].done
            // server do something.
        }(i)
    }

    time.Sleep(1 * time.Second)

    for i := 0; i < 10; i   {
        // event happen.
        s.events[i].done <- true
    }
}

Alternatively don't access the map in the go routine:

package main

import (
    "sync"
    "time"
)

type event struct {
    done chan bool
}

type server struct {
    mu     sync.Mutex
    events map[int]*event
}

func main() {
    s := server{}
    s.events = make(map[int]*event)

    for i := 0; i < 10; i   {
        s.events[i] = &event{}
        c := make(chan bool)
        s.events[i].done = c

        go func(i int, c chan bool) {
            time.Sleep(1 * time.Millisecond)
            <-c
            // server do something.
        }(i, c)
    }

    time.Sleep(1 * time.Second)

    for i := 0; i < 10; i   {
        // event happen.
        s.events[i].done <- true
    }
}

In the comments you asked about dealing with a situation where you don't know the number of events. The solution is going to depend on the situation but here is one way I've used to deal with similar situations (this appears complicated but I think its easier to follow then using a map and surrounding every access in a Mutex).

package main

import (
    "sync"
    "time"
)

type event struct {
    done chan bool
}

type server struct {
    events map[int]*event
}

func main() {
    s := server{}
    s.events = make(map[int]*event)

    // Routine to trigger channels
    triggerChan := make(chan chan bool) // Send new triggers to this...
    eventChan := make(chan struct{})    // Close this when the event happens and go routines should continue
    go func() {
        var triggers []chan bool
        eventReceived := false
        for {
            select {
            case t, ok := <-triggerChan:
                if !ok { // You want some way for the goRoutine to shut down - in this case we wait on the closure of triggerChan
                    return
                }
                if eventReceived {
                    t <- true // The event has already happened so go routine can proceed immediately
                } else {
                    triggers = append(triggers, t)
                }
            case <-eventChan:
                for _, c := range triggers {
                    c <- true
                }
                eventReceived = true
                eventChan = nil // Don't want select to be triggered again...
            }
        }
    }()

    // Start up the event handlers...
    var wg sync.WaitGroup
    wg.Add(10)
    for i := 0; i < 10; i   {
        s.events[i] = &event{}
        c := make(chan bool)
        triggerChan <- c

        go func(i int, c chan bool) {
            time.Sleep(1 * time.Millisecond)
            <-c
            // server do something.
            wg.Done()
        }(i, c)
    }

    time.Sleep(1 * time.Second)

    // Event happened - release the go routines
    close(eventChan)
    wg.Wait()
    close(triggerChan)
}