CODEWITHSUNDEEP
go
codingenious
codingenious

Reputation: 8653

Double checked locking in golang - Why mutex.RLock() is required?

I have a piece of code from this website which has double checked locking for initialization of an object.

func checkSyncProducer() {
    mutex.RLock()
    if syncProducer == nil {
        mutex.RUnlock()
        mutex.Lock()
        defer mutex.Unlock()
        if syncProducer == nil {
            syncProducer  = createSyncKafkaProducer() //this func will initialize syncProducer.
        }
    } else {
        defer mutex.RUnlock()
    }
}

This piece of code has mutex.RLock() before first nil check.

Why is that required? (it is explained in the page but I couldn't understand) And doesn't it add overhead becuase everytime checkSyncProducer is called read lock will be taken and released.

Should there be one more nil check before acquiring read lock like:

func checkSyncProducer() {
    if syncProducer == nil {
        mutex.RLock()
        if syncProducer == nil {
            mutex.RUnlock()
            mutex.Lock()
            defer mutex.Unlock()
            if syncProducer == nil {
                createSyncKafkaProducer()
            }
        } else {
            defer mutex.RUnlock()
        }
    }
}

First nil check will ensure that RLock is not taken unnecessarily. Am I correct?

Upvotes: 5

Views: 3205

Answers (2)

Paul Hankin
Paul Hankin

Reputation: 58339

If you don't acquire a RLock to read syncProducer, it's a data race, since another goroutine may update it.

If you assume reads/writes to pointer variables are atomic, this looks harmless -- the racy read of syncProducer can never cause incorrect behavior. If you don't make this atomic assumption, the read could produce just some of the bytes of the pointer if you're unlucky, and your program would crash.

It may or may not be possible depending on the architecture, machine word size, compiler version, etc. that you're using. But the RLock avoids any concern.

Rather than explicitly mess around with RWLocks, it's probably better to use this (assuming the goal is to lazily initialize a variable):

var (
    syncOnce sync.Once
    syncProducerInternal *syncProducerType
)

func syncProducer() *syncProducerType {
    syncOnce.Do(func() { syncProducerInternal = createSyncKafkaProducer() })
    return syncProducerInternal
}

Then code that needs the sync producer can call the syncProducer() function to get it, and never see a nil pointer.

Upvotes: 14

Jonathan Hall
Jonathan Hall

Reputation: 79704

It's required because the check is a read-only operation. Doing a RW lock instead would be possible, but would mean that only one goroutine at a time could do the check.

Upvotes: 2

Related Questions