-
Notifications
You must be signed in to change notification settings - Fork 2.5k
Closed
Description
After creating channel using PubSub.Channel()
, subsequent calls to PubSub.Unsubscribe()
may result in data race.
Expected Behavior
There shouldn't be a data race.
Current Behavior
In initChannel()
, there is a goroutine that periodically calls PubSub.Ping()
, which causes data race when there is a separate call to PubSub.Unsubscribe()
in another goroutine.
Possible Solution
Perhaps acquire the mutex lock in Ping()
too:
c.mu.Lock()
defer c.mu.Unlock()
Steps to Reproduce
Please see the minimal working example to recreate the race condition: YongJieYongJie@115dcfc
- Checkout the branch above
- Run
go mod init github.com/go-redis/redis && go mod tidy
- Run
GORACE="halt_on_error=1" go test -v -race -run TestDataRaceRecreate
- The test should result in error like the following:
==================
WARNING: DATA RACE
Read at 0x00c000021b28 by goroutine 11:
bufio.(*Writer).Flush()
/usr/local/go/src/bufio/bufio.go:608 +0x16b
github.com/go-redis/redis/internal/proto.(*Writer).Flush()
/<TRUNCATED>/go-redis/redis/internal/proto/writer.go:158 +0xc7
github.com/go-redis/redis/internal/pool.(*Conn).WithWriter()
/<TRUNCATED>/go-redis/redis/internal/pool/conn.go:86 +0x90
github.com/go-redis/redis.(*PubSub).writeCmd()
/<TRUNCATED>/go-redis/redis/pubsub.go:87 +0xb4
github.com/go-redis/redis.(*PubSub).Ping()
/<TRUNCATED>/go-redis/redis/pubsub.go:261 +0x1ec
github.com/go-redis/redis.(*PubSub).initChannel.func2()
/<TRUNCATED>/go-redis/redis/pubsub.go:498 +0x2c6
Previous write at 0x00c000021b28 by goroutine 17:
bufio.(*Writer).WriteByte()
/usr/local/go/src/bufio/bufio.go:666 +0x144
github.com/go-redis/redis/internal/proto.(*Writer).WriteArgs()
/<TRUNCATED>/go-redis/redis/internal/proto/writer.go:30 +0x54
github.com/go-redis/redis.writeCmd()
/<TRUNCATED>/go-redis/redis/command.go:46 +0x88
github.com/go-redis/redis.(*PubSub).writeCmd.func1()
/<TRUNCATED>/go-redis/redis/pubsub.go:88 +0x7c
github.com/go-redis/redis/internal/pool.(*Conn).WithWriter()
/<TRUNCATED>/go-redis/redis/internal/pool/conn.go:85 +0x7b
github.com/go-redis/redis.(*PubSub).writeCmd()
/<TRUNCATED>/go-redis/redis/pubsub.go:87 +0xb4
github.com/go-redis/redis.(*PubSub)._subscribe()
/<TRUNCATED>/go-redis/redis/pubsub.go:131 +0x353
github.com/go-redis/redis.(*PubSub).subscribe()
/<TRUNCATED>/go-redis/redis/pubsub.go:244 +0x104
github.com/go-redis/redis.(*PubSub).Unsubscribe()
/<TRUNCATED>/go-redis/redis/pubsub.go:221 +0x1ba
github.com/go-redis/redis_test.TestRedisBug.func2()
/<TRUNCATED>/go-redis/redis/race_test.go:372 +0xea
Goroutine 11 (running) created at:
github.com/go-redis/redis.(*PubSub).initChannel()
/<TRUNCATED>/go-redis/redis/pubsub.go:482 +0x190
github.com/go-redis/redis.(*PubSub).channel.func1()
/<TRUNCATED>/go-redis/redis/pubsub.go:415 +0x4a
sync.(*Once).doSlow()
/usr/local/go/src/sync/once.go:66 +0x109
sync.(*Once).Do()
/usr/local/go/src/sync/once.go:57 +0x68
github.com/go-redis/redis.(*PubSub).channel()
/<TRUNCATED>/go-redis/redis/pubsub.go:414 +0x7e
github.com/go-redis/redis.(*PubSub).Channel()
/<TRUNCATED>/go-redis/redis/pubsub.go:404 +0x17a
github.com/go-redis/redis_test.TestRedisBug()
/<TRUNCATED>/go-redis/redis/race_test.go:355 +0x168
testing.tRunner()
/usr/local/go/src/testing/testing.go:1123 +0x202
Goroutine 17 (running) created at:
github.com/go-redis/redis_test.TestRedisBug()
/<TRUNCATED>/go-redis/redis/race_test.go:369 +0x404
testing.tRunner()
/usr/local/go/src/testing/testing.go:1123 +0x202
Context (Environment)
Detailed Description
Possible Implementation
whunmr, wafeishushu, updogliu, SmileYun and wangfengfly
Metadata
Metadata
Assignees
Labels
No labels