Search code examples
gogoroutine

Is this code thread-safe, given that it allows dirty reads?


Is this code thread-safe, given that it allows dirty reads? It's not a map, it's a pointer to a struct.

I want to know if this is safe, allowing old variables to be read, is it safe if it is a string or a num?

package main

import (
    "fmt"
    "golang.org/x/time/rate"
    "sync"
)

var (
    TestLimiter = rate.NewLimiter(1000, 1500)
)

func UpdateLimiter(funcName string, limit int, burst int) {
    switch funcName {
    case "Test":
        TestLimiter = rate.NewLimiter(rate.Limit(limit), burst)
    }
}

func main() {
    var wg sync.WaitGroup
    for i := 0; i < 100; i++ {
        wg.Add(1)
        if i == 4 {
            go func() {
                UpdateLimiter("Test", 1000, 1500)
                wg.Done()
            }()
            continue
        }
        go func() {
            if TestLimiter.Allow() {
                fmt.Println("allow")
            } else {
                fmt.Println("not allow")
            }

            wg.Done()
        }()
    }
    wg.Wait()
    fmt.Println("done")

}

I tried to run it locally, and I didn't notice any panic. run with go run main.go

but when i run go run -race main.go, it's show Found 7 data race(s) exit status 66


Solution

  • What do you mean "thread safe"? It's clearly not free of data races. The go memory model provides guarantees about your program: arbitrary misbehavior is not allowed in your program (partly because you're updating a pointer, which will be a machine word in size), but possible behaviors are that the program can terminate if it notices a data-race and it may never "see" the updates to TestLimiter.

    If you don't want to pay the (usually insignificant) synchronization costs of a mutex, use an atomic.Pointer[rate.Limiter] instead of a plain pointer, and use its Load and Store methods to read and update the pointer.

    It's not your question, but personally I would use synchronization here. It's very unlikely that the cost of the mutex will be significant compared to the cost of Allow method call. You can use code like this:

    type rateLimiter struct {
      mut sync.Mutex
      l *rate.Limiter
    }
    
    func (rl *rateLimiter) set(l *rate.Limiter) {
      rl.mut.Lock()
      defer rl.mut.Unlock()
      rl.l = l
    }
    
    func (rl *rateLimiter) get() *rate.Limiter {
      rl.mut.Lock()
      defer rl.mut.Unlock()
      return rl.l
    }
    
    func (rl *rateLimiter) Allow() bool {
        return rl.get().Allow()
    }
    
    var TestLimiter = &rateLimiter{l:  rate.NewLimiter(1000, 1500)}
    

    and then use the set() method when you want to swap out the rate limiter.