Is it possible to have race condition with golang chan?

Question

My goal is to create a simple websocket server. I use chan to distribute the message e.g by invoking <-messageChan. the messageChan have many writers and readers.

However, this StackOverflow question scares me of causing an unintentionally deadlock.

What I've did:

1. Create a test that essentially do:
   1. populate a chan int with 0 to 1000.
   2. create 100 goroutine to invoke <-chan and add it to a map[int]bool.
   3. invoke t.Fatal if len(map[int]bool) is not 1000. In other words, race condition.

However, the test did not fail. I am afraid, I did something wrong, and chan can have deadlock.

The code

main_test.go

package main

import (
    "log"
    "sync"
    "testing"
)

type MapMux struct {
    sync.RWMutex
    m map[int]bool
    sync.WaitGroup
}

func (mux *MapMux) AddInt(i int) {
    mux.RLock()
    if _, isExist := mux.m[i]; isExist {
        log.Fatal("race condition")
    }
    mux.RUnlock()
    mux.Lock()
    mux.m[i] = true
    mux.Unlock()
    mux.Done()
}

func TestChanRaceCondition(t *testing.T) {
    l := 1000
    c := make(chan int, l)
    defer close(c)
    for i := 0; i < l; i++ {
        c <- i
    }

    mux := MapMux{sync.RWMutex{}, map[int]bool{}, sync.WaitGroup{}}

    mux.Add(l)

    for i := 0; i < 100; i++ {
        go func(key int) {
            for {
                payload := <-c
                log.Printf("go%d: %d", key, payload)
                mux.AddInt(payload)
            }
        }(i)
    }

    mux.Wait()

    if len(mux.m) != l {
        t.Fatal("expected len:", l, ", actual len:", len(mux.m))
    }
}

Edit:

1. The code finds duplicates in the map[int]bool field. Edit: This is because defer close(c). I should have check is the channel was open for every operation.

Anyway, this is what I learn. Hope this help new Golangers.

Lesson learnt:

1. it is okay to have many writers and many readers for one channel.
2. always check val, isOpen := <-chan. If isOpen == false, then stop using the channel.
3. sync.RWMutex.RLock() does not guarantee other goroutine from making changes to the map. So, becareful. This is how it should be done.

    func (mux *MapMux) AddInt(i int) {
        mux.RLock()
        if _, isExist := mux.m[i]; isExist {
            log.Fatal("race condition")
        }
        mux.RUnlock()
        mux.Lock()
        if _, isExist := mux.m[i]; isExist {
            log.Fatal("race condition")
        }
        mux.m[i] = true
        mux.Unlock()
        mux.Done()
    }
   

Answer 1

A data race happens if you share memory between goroutines. If you do not share memory between goroutines, the kind of data races you are afraid of will not happen. There can be other race conditions, such as the one you have in your AddInt method. The correct version should be:

func (mux *MapMux) AddInt(i int) {
    mux.RLock()
    if _, isExist := mux.m[i]; isExist {
        log.Fatal("race condition")
    }
    mux.RUnlock()
    mux.Lock()
    if _, isExist := mux.m[i]; isExist {
        log.Fatal("race condition")
    }
    mux.m[i] = true
    mux.Unlock()
    mux.Done()
}

This is because there is no guarantee that another goroutine changes the map between RUnlock and Lock.

In your example, you are sharing a map between goroutines. That means you have to protect all access to it using a mutex. If you do that, there will be no data races around the use of that map.

However, in general, if you can avoid sharing memory, your program will not have such data races.