Search code examples
goconcurrencymutexgoroutine

Why GO panics with 'concurrent map writes' even with locks in place?


When trying to use this struct with multiple goroutines sometimes I get one of these errors:

fatal error: concurrent map read and map write

or

concurrent map writes

After reading the this thread I made sure to return a reference in the constructor and pass in a reference to the receivers.

The entirety of the code where this is being used is in this github repo

type concurrentStorage struct {
    sync.Mutex
    domain string
    urls map[url.URL]bool
}

func newConcurrentStorage(d string) *concurrentStorage{
    return &concurrentStorage{
        domain: d,
        urls: map[url.URL]bool{},
    }
}

func (c *concurrentStorage) add(u url.URL) (bool) {
    c.Lock()
    defer c.Unlock()
    if _, ok := c.urls[u]; ok{
        return false
    }
    c.urls[u] = true
    return true
}

Solution

  • Upon reading the code on Github that you linked to, the crawl() function accepts a concurrentStorage (not a pointer).

    For each de-reference (ie: *urlSet) when calling crawl(), you are copying the concurrentStorage struct (including the sync.Mutex) while the map retains the pointer to the original. This means that your mutexes are isolated to each goroutine, while they are sharing the same state.

    If you change crawl() to accept a pointer instead, and stop de-referencing concurrentStorage, it will work as you intend.