tb icon indicating copy to clipboard operation
tb copied to clipboard

Potential problem in throttler Bucket function

Open echinmay opened this issue 10 years ago • 1 comments

// Bucket returns a Bucket with rate capacity, keyed by key. // // If a Bucket (key, rate) doesn't exist yet, it is created. // // You must call Close when you're done with the Throttler in order to not leak // a go-routine and a system-timer. func (t *Throttler) Bucket(key string, rate int64) *Bucket { t.mu.RLock() b, ok := t.buckets[key] t.mu.RUnlock()

if !ok {
    b = NewBucket(rate, -1)
    b.inc = int64(math.Floor(.5 + (float64(b.capacity) * t.freq.Seconds())))
    b.freq = t.freq
    t.mu.Lock()
    t.buckets[key] = b
    t.mu.Unlock()
}

return b

}

Is there a possibility of two routines checking for t.buckets[key], not finding it and then allocating a NewBucket?

I think the lock should extend for the whole function. Something like.

func (t *Throttler) Bucket(key string, rate int64) *Bucket { t.mu.Lock() defer(t.mu.Lock())

b, ok := t.buckets[key]
if !ok {
    b = NewBucket(rate, -1)
    b.inc = int64(math.Floor(.5 + (float64(b.capacity) * t.freq.Seconds())))
    b.freq = t.freq
    t.buckets[key] = b
}

return b

}

echinmay avatar Dec 03 '15 08:12 echinmay

It seems like you're right! Good catch! Please open a PR with the change :-)

tsenart avatar Dec 03 '15 21:12 tsenart