How to eliminate Data Race even with Mutex Locks around pointer variable-CodePudding

Some starter code goes here,

func (chm *ConcurrentHashMap) NFetchWorker() {
    for {
        key := <-NFetchWorkerPipe
        chm.mu.RLock()
        data := chm.data[string(key)]
        chm.mu.RUnlock()
        if data.IsUsingNFetch {
            chm.mu.Lock()
            *(chm.data[string(key)].NFetch)--
            chm.mu.Unlock()
        }
    }
}

go NFetchWorker()

Struct ConcurrentHashMap looks like this,

type ConcurrentHashMap struct {
    data map[string]DataBlock
    mu sync.RWMutex
}

Struct DataBlock looks like this,

type DataBlock struct {
    ...
    NFetch        *int32
    IsUsingNFetch bool
    ...
}

Now when I try to run tests with race flag enabled. I get,

Write at 0x00c00012e310 by goroutine 8:
  (*ConcurrentHashMap).NFetchWorker()

The line numbers point to this line.

*(chm.data[string(key)].NFetch)--

However I don't face this issue when I do not use pointer in DataBlock.NFetch. But if I do that I lose the ability to make changes to NFetch directly from map without reassigning a whole new struct object to that hash in map, which would be relatively computationally expensive. I want to change the value of NFetch without reassigning the whole struct again for one small change while being free from DATA RACE. Any solutions??

Fairly New to Golang, I could be doing something really stupid here, feel free to point it out.

UPDATE: Adding Read and Write Op function

func (chm *ConcurrentHashMap) Get(key Key) Value {
    chm.mu.RLock()
    fetch, ok := chm.data[string(key)]
    chm.mu.RUnlock()
    if ok {
        if CheckValueValidity(&fetch) {
            NFetchWorkerPipe <- key
            return fetch.Value
        } else {
            chm.mu.Lock()
            delete(chm.data, string(key))
            chm.mu.Unlock()
        }
    }

    return constants.NIL
}

Write Op

func (chm *ConcurrentHashMap) Insert(key Key, value Value, options Options) {
    ...
    chm.mu.Lock()
    chm.data[string(key)] = Block{
        Value:         value,
        NFetch:        nFetchOld,
        Expiry:        time.Now().Add(delay),
        IsUsingNFetch: foundNFetch,
        IsUsingExpiry: foundTTL,
        mu:            mutex,
    }
    chm.mu.Unlock()
}

CodePudding user response：

The problem is in the CheckValueValidity where you are accessing NFetch without locking it. Pointers should never be accessed without locking them.

func CheckValueValidity(value *Block) bool {
    if value.IsUsingExpiry && !value.Expiry.After(time.Now()) {
        return false
    }

    if value.IsUsingNFetch && *(value.NFetch) <= 0 {
        return false
    }

    return true
}

This code should work

func (chm *ConcurrentHashMap) Get(key Key) Value {
    chm.mu.RLock()
    fetch, ok := chm.data[string(key)]
    isValid := CheckValueValidity(&fetch)
    chm.mu.RUnlock()
    if ok {
        if isValid {
            NFetchWorkerPipe <- key
            return fetch.Value
        } else {
            chm.mu.Lock()
            delete(chm.data, string(key))
            chm.mu.Unlock()
        }
    }

    return constants.NIL
}