fix: handle panic in ringShards Hash function when Ring got closed

Fixes #2126
This commit is contained in:
Max Riveiro 2022-07-13 18:09:42 +03:00
parent 092a692384
commit a80b84f01f
2 changed files with 27 additions and 14 deletions

26
ring.go
View File

@ -259,10 +259,11 @@ func (c *ringShards) Hash(key string) string {
var hash string var hash string
c.mu.RLock() c.mu.RLock()
defer c.mu.RUnlock()
if c.numShard > 0 { if c.numShard > 0 {
hash = c.hash.Get(key) hash = c.hash.Get(key)
} }
c.mu.RUnlock()
return hash return hash
} }
@ -271,27 +272,22 @@ func (c *ringShards) GetByKey(key string) (*ringShard, error) {
key = hashtag.Key(key) key = hashtag.Key(key)
c.mu.RLock() c.mu.RLock()
defer c.mu.RUnlock()
if c.closed { if c.closed {
c.mu.RUnlock()
return nil, pool.ErrClosed return nil, pool.ErrClosed
} }
if c.numShard == 0 { if c.numShard == 0 {
c.mu.RUnlock()
return nil, errRingShardsDown return nil, errRingShardsDown
} }
hash := c.hash.Get(key) hash := c.hash.Get(key)
if hash == "" { if hash == "" {
c.mu.RUnlock()
return nil, errRingShardsDown return nil, errRingShardsDown
} }
shard := c.shards[hash] return c.shards[hash], nil
c.mu.RUnlock()
return shard, nil
} }
func (c *ringShards) GetByName(shardName string) (*ringShard, error) { func (c *ringShards) GetByName(shardName string) (*ringShard, error) {
@ -300,9 +296,9 @@ func (c *ringShards) GetByName(shardName string) (*ringShard, error) {
} }
c.mu.RLock() c.mu.RLock()
shard := c.shards[shardName] defer c.mu.RUnlock()
c.mu.RUnlock()
return shard, nil return c.shards[shardName], nil
} }
func (c *ringShards) Random() (*ringShard, error) { func (c *ringShards) Random() (*ringShard, error) {
@ -357,9 +353,9 @@ func (c *ringShards) rebalance() {
func (c *ringShards) Len() int { func (c *ringShards) Len() int {
c.mu.RLock() c.mu.RLock()
l := c.numShard defer c.mu.RUnlock()
c.mu.RUnlock()
return l return c.numShard
} }
func (c *ringShards) Close() error { func (c *ringShards) Close() error {
@ -377,8 +373,10 @@ func (c *ringShards) Close() error {
firstErr = err firstErr = err
} }
} }
c.hash = nil c.hash = nil
c.shards = nil c.shards = nil
c.numShard = 0
c.list = nil c.list = nil
return firstErr return firstErr

View File

@ -114,6 +114,21 @@ var _ = Describe("Redis Ring", func() {
}) })
Describe("pipeline", func() { Describe("pipeline", func() {
It("doesn't panic closed ring, returns error", func() {
pipe := ring.Pipeline()
for i := 0; i < 3; i++ {
err := pipe.Set(ctx, fmt.Sprintf("key%d", i), "value", 0).Err()
Expect(err).NotTo(HaveOccurred())
}
Expect(ring.Close()).NotTo(HaveOccurred())
Expect(func() {
_, execErr := pipe.Exec(ctx)
Expect(execErr).To(HaveOccurred())
}).NotTo(Panic())
})
It("distributes keys", func() { It("distributes keys", func() {
pipe := ring.Pipeline() pipe := ring.Pipeline()
for i := 0; i < 100; i++ { for i := 0; i < 100; i++ {