gomodule / redigo

Go client for Redis
Apache License 2.0
9.76k stars 1.25k forks source link

panic: runtime error: slice bounds out of range #630

Closed XiaoyuShi6 closed 1 year ago

XiaoyuShi6 commented 2 years ago

Hi, I guess I meet the same concurrency bug as https://github.com/gomodule/redigo/issues/540. This is the panic log:

panic: runtime error: slice bounds out of range [15:10]

goroutine 16 [running]:
bufio.(*Reader).ReadSlice(0xc00007b200, 0x8a?)
        /home/work/go_install/go/src/bufio/bufio.go:346 +0x225
github.com/gomodule/redigo/redis.(*conn).readLine(0xc0004dbac0?)
        /home/work/go/pkg/mod/github.com/gomodule/redigo@v2.0.0+incompatible/redis/conn.go:431 +0x26
github.com/gomodule/redigo/redis.(*conn).readReply(0xc00058e8c0)
        /home/work/go/pkg/mod/github.com/gomodule/redigo@v2.0.0+incompatible/redis/conn.go:504 +0x25
github.com/gomodule/redigo/redis.(*conn).DoWithTimeout(0xc00058e8c0, 0x0, {0xba10eb, 0x3}, {0xc0005888c0, 0x5, 0x5})
        /home/work/go/pkg/mod/github.com/gomodule/redigo@v2.0.0+incompatible/redis/conn.go:665 +0x3cf
github.com/gomodule/redigo/redis.(*conn).Do(0xaee440?, {0xba10eb?, 0xba10eb?}, {0xc0005888c0?, 0x10?, 0xabf780?})
        /home/work/go/pkg/mod/github.com/gomodule/redigo@v2.0.0+incompatible/redis/conn.go:616 +0x39
github.com/gomodule/redigo/redis.(*activeConn).Do(0xc000010558, {0xba10eb, 0x3}, {0xc0005888c0, 0x5, 0x5})
        /home/work/go/pkg/mod/github.com/gomodule/redigo@v2.0.0+incompatible/redis/pool.go:447 +0xea

I want to use redigo to realize a distributed lock.After get the lock, create a new goroutine to update the expire time. Everywhere I use redis, I will get a new client from the redisPool and close it after using. However, I still meet this problem.Here is my code:

lock


func (r *RedisLock) Lock() (err error) {
for {
if err = r.grant(); err != nil {
time.Sleep(100 * time.Millisecond)
} else {
break
}
}
log.Debug("redis key lock success, key: %s", r.key)
ctx, cancelFun := context.WithCancel(context.TODO())
r.cancelFun = cancelFun
r.updateExpire(ctx)
r.isLocked = true
return nil
}

func (r *RedisLock) grant() (err error) { grantClient := redisClient.Get() defer grantClient.Close() res, err := redis.String(grantClient.Do("SET", r.key, r.lockId, "EX", r.ttl, "NX")) if res != "OK" { err = errors.New(fmt.Sprintf("grant res is not OK: %s", res)) return err }

return nil

}

> update expire

func (r *RedisLock) updateExpire(ctx context.Context) { expireClient := redisClient.Get() defer expireClient.Close() go func() { for { select { case <-ctx.Done(): return default: res, err := redis.Int(expireClient.Do("EXPIRE", r.key, r.ttl)) log.Debug("updateExpire") if res != 1 { if err != nil { log.Error("update redis lock expire error: %s,key : %s", err.Error(), r.key) } else { log.Error("update redis lock expire error,key : %s", r.key) } }

        }
        time.Sleep(time.Duration(r.ttl/2) * time.Second)
    }
}()

}

> unlock

func (r *RedisLock) UnLock(lockId string) (err error) { unlockClient := redisClient.Get() defer unlockClient.Close() if r.cancelFun != nil { r.cancelFun() } if r.isLocked { if lockId != r.lockId { err = errors.New(fmt.Sprintf("lockid is different, can't unlock, key: %s", r.key)) } else { res, _ := redis.Int(unlockClient.Do("DEL", r.key)) if res != 1 { return errors.New(fmt.Sprintf("redis unlock error,key: %s", r.key)) } } } return }


Thanks !
stevenh commented 1 year ago

Your updateExpire is broken as it returns the connection to the pool while the goroutine is still using it.

You want:


func (r *RedisLock) updateExpire(ctx context.Context) {
    expireClient := redisClient.Get()
    go func() {
        defer expireClient.Close()
        for {
            select {
            case <-ctx.Done():
                return
            default:
                res, err := redis.Int(expireClient.Do("EXPIRE", r.key, r.ttl))
                log.Debug("updateExpire")
                if res != 1 {
                    if err != nil {
                        log.Error("update redis lock expire error: %s,key : %s", err.Error(), r.key)
                    } else {
                        log.Error("update redis lock expire error,key : %s", r.key)
                    }
                }

            }
            time.Sleep(time.Duration(r.ttl/2) * time.Second)
        }
    }()
}