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

Question

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?

hmm, I'd be interested what people have to say - doesnt look like that RLock is needed to me, the nil check is sufficient - perhaps Im missing something, it may depend on wider context but I think as long as you check for nil, aquire the lock and then check it is still nil (in case another thread initialised in that microsecond) it should be fine — SwiftD, Jan 08 '19 at 13:49
I don't understand the last sentence. Why do you think there should be an additional nil check? — Jonathan Hall, Jan 08 '19 at 13:51
thinking another thread could initialise in the split second between the check for nil and aquiring the lock, strangley though you seem to have suggested the rlock IS needed in your answer — SwiftD, Jan 08 '19 at 13:52
Oh, you mean between `mutext.Unlock()` and `mutex.Lock()`? That's not a problem, because if a lock is established by another goroutine at thta juncture, the `Lock` call will block until all other locks are released. — Jonathan Hall, Jan 08 '19 at 13:56
Without locking double-checked "locking" doesn't work. Maybe if you carefully used memory barriers. One possible error is lack of ordering. The pointer can be set but the object not initialized. Even if the other thread initialized it before setting the pointer. — Zan Lynx, Jan 08 '19 at 13:57
Sorry, no this is what i meant, see here (not working but shows what i mean) with no need for rlock - https://play.golang.org/p/_P3-8r4LQfz — SwiftD, Jan 08 '19 at 13:59
@Flimzy What I mean by last statement is - Once syncProducer is initialized and when next time `checkSyncProducer` is called, func will take RLock even though it is not required now. Adding one more nil check before taking RLock will prevent this, right? — codingenious, Jan 08 '19 at 14:05
I edited question. Is second piece of code better than first in terms of performance given function is called multiple times. — codingenious, Jan 08 '19 at 14:11
That second implementation is susceptible to the race condition that the lock is supposed to prevent. `syncProducer` must not be read without holding at least a read lock, and must not be assigned without holding the write lock. — Peter, Jan 08 '19 at 14:13
The second code block will crash, due to the race condition. — Jonathan Hall, Jan 08 '19 at 14:17
Yeah. Now I understand it. And it seems to me now that question itself was sore. :) — codingenious, Jan 08 '19 at 14:28

Paul Hankin · Accepted Answer · 2019-01-08T14:46:28.477

11

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.

edited Jan 08 '19 at 14:46

answered Jan 08 '19 at 14:24

Paul Hankin

54,811
11
92
118

1

see this SO question for an explanation: go assignments are not guaranteed to be atomic: https://stackoverflow.com/questions/34750323/is-variable-assignment-atomic-in-go – thst Jan 08 '19 at 14:27
i dont think that would apply to a nil check - something is either nil or not it cant be part nil – SwiftD Jan 08 '19 at 14:31
2

@SwiftD the pointer can be partly written to RAM as it's read. Then you might get some 0 bytes, and some pointer bytes. So not nil, but not a valid pointer either. As I say in the question, whether this can actually happen or not depends on the architecture etc. It's still probably OK if you acquire a read lock later when you read the variable, but I guess the point of this code is to avoid that. – Paul Hankin Jan 08 '19 at 14:36
2

Good explanation of the locking https://launchdarkly.com/blog/golang-pearl-thread-safe-writes-and-double-checked-locking-in-go/ – thst Jan 08 '19 at 14:45
@thst thanks for that -- I'd just added a sync.Once example in my answer independently, but I see the article suggests it at the end... – Paul Hankin Jan 08 '19 at 14:48

score 2 · Answer 2 · answered Jan 08 '19 at 13:51

2

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.

answered Jan 08 '19 at 13:51

Jonathan Hall

75,165
16
143
189

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

2 Answers2