Golang data race cause by concurrent map read

Question

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?

Does this answer help? (relevant release note states "if one goroutine is writing to a map, no other goroutine should be reading or writing the map concurrently") Your Mutex is preventing simultaneous writes but a read can still happen at the same time as a write (read at line 29, write at line 24). — Brits
– Brits, Commented Feb 26, 2022 at 2:55
@Brits I have read this answer. But if I use Mutex to prevent read simultaneous, it may cause deadlock, because the read goroutine will not release lock until receiving data from channel... — eye_water
– eye_water, Commented Feb 26, 2022 at 3:00

Brits · Accepted Answer · 2022-02-26 03:47:51Z

4

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)
}

edited Feb 26, 2022 at 3:47

answered Feb 26, 2022 at 3:06

Brits

18.9k3 gold badges26 silver badges44 bronze badges

Sign up to request clarification or add additional context in comments.

5 Comments

eye_water Over a year ago

If I don't know how many events there are, I only know to create an event after a request, and then monitor the event through the channel. What to do in this situation? I think I know how to do it, don't access the map in the go routine! Thanks a lot!

erik258 Over a year ago

So an incoming request creates an event, which then processes asynchronously. Does anything need to access the event in the mean time?

Brits Over a year ago

Hard to answer without a real example. One solution I've used is to start a separate go routine which accepts channels on a channel (i.e. chan chan bool) and then does whatever it needs to with the received channels (e.g. stores them in a map and then sends stuff to them based on data received from other channels).

eye_water Over a year ago

@DanielFarrell Yeah, I think I think my description is not clear enough. If the server receives a request from a certain user, if this user's event hasn't been created, create it. If this user's event has been created, check if the new request satisfies the conditions of event done.

eye_water Over a year ago

@Brits After I tried it, I used to create channel in a for loop and then pass channel in the goroutine. This does what I need. Thanks a lot.

Collectives™ on Stack Overflow

Golang data race cause by concurrent map read

1 Answer 1

5 Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

5 Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related