CODEWITHSUNDEEP
gomutex
manticeo
manticeo

Reputation: 27

Is it a good practice to lock some data outside if-else and unlock inside if-else

When I test rf.state == candidate, there could be a race. So I lock the struct rf before that. I want to make sure rf.state appears exactly in the if condition test so I can use its most current value.

...
func (rf *Raft) runElection() {
    rf.mu.Lock()
    rf.currentTerm += 1
    rf.votedFor = rf.me
    rf.votes = 1
    rf.mu.Unlock()

    for server := range rf.peers {
        rf.mu.Lock()
        if server != rf.me && rf.state == candidate {
            rf.mu.Unlock()
            go rf.sendRequestVote(server)
        } else {
            rf.mu.Unlock()
        }
    }
}

...

Is there any other idiomatic way to lock the data in similar situations (if-else, switch...)?

EDITS: Here is some other code of mine.

...
rf.mu.Lock()
switch rf.state {
case follower:
        rf.mu.Unlock()

        select {
        case <-rf.chanAppends:
        case <-rf.chanGrantVotes:
        case <-time.After(time.Duration(500+rand.Intn(1000)) * time.Millisecond):
                rf.mu.Lock()
                rf.state = candidate
                rf.mu.Unlock()
        }
case candidate:
        rf.mu.Unlock()
        ...
case leader:
        rf.mu.Unlock()
        ...

To use the current rf.state upon switch, I have to lock at the outside and unlock it at the inside. What bothers me is that for every case I have to unlock for every cases (making the code less clean and readable?). And there is some blocking (e.g. time.After(...)) going on to make a timer inside every case so I see no way of using defer to help me. I guess it's some tradeoff I have to make if I don't want to alter the logical structure of this switch?

Upvotes: 1

Views: 322

Answers (1)

mhutter
mhutter

Reputation: 2916

While your code looks okay, you can always extract parts into their own functions to make use of defer.

func (rf *Raft) runElection() {
    // ...

    for server := range rf.peers {
        tryVote(rf, server)
    }
}

func tryVote(rf *Raft, server xxx) {
    rf.mu.Lock()
    defer rf.mu.Unlock()
    if server != rf.me && rf.state == candidate {
        go rf.sendRequestVote(server)
    }
}

This is especially helpful when your code gets more complex and you want to make ABSOLUTELY sure the mutex gets unlocked once you return from some code section.

Upvotes: 1

Related Questions