From 7b2d018f847e9d97a8c0901641fb6405b03481dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gustavo=20Cha=C3=ADn?= Date: Thu, 22 Aug 2019 11:23:07 +0200 Subject: [PATCH] rpc: protect subscription access from race condition (#3910) When using the RPC client in my test suite (with -race enabled), I do a lot of Subscribe/Unsubscribe operations, at some point (randomly) the race detector returns the following warning: WARNING: DATA RACE Read at 0x00c0009dbe30 by goroutine 31: runtime.mapiterinit() /usr/local/go/src/runtime/map.go:804 +0x0 github.com/tendermint/tendermint/rpc/client.(*WSEvents).redoSubscriptionsAfter() /go/pkg/mod/github.com/tendermint/tendermint@v0.31.5/rpc/client/httpclient.go:364 +0xc0 github.com/tendermint/tendermint/rpc/client.(*WSEvents).eventListener() /go/pkg/mod/github.com/tendermint/tendermint@v0.31.5/rpc/client/httpclient.go:393 +0x3c6 Turns out that the redoSubscriptionAfter is not protecting the access to subscriptions. The following change protects the read access to the subscription map behind the mutex --- CHANGELOG_PENDING.md | 1 + rpc/client/httpclient.go | 2 ++ 2 files changed, 3 insertions(+) diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index aadc3500..39be1c69 100644 --- a/CHANGELOG_PENDING.md +++ b/CHANGELOG_PENDING.md @@ -28,3 +28,4 @@ program](https://hackerone.com/tendermint). - [config] \#3868 move misplaced `max_msg_bytes` into mempool section - [store] \#3893 register block amino, not just crypto +- [rpc] [\#3910](https://github.com/tendermint/tendermint/pull/3910) protect subscription access from race conditions (@gchaincl) diff --git a/rpc/client/httpclient.go b/rpc/client/httpclient.go index 85f065b6..c4334523 100644 --- a/rpc/client/httpclient.go +++ b/rpc/client/httpclient.go @@ -453,6 +453,8 @@ func (w *WSEvents) UnsubscribeAll(ctx context.Context, subscriber string) error func (w *WSEvents) redoSubscriptionsAfter(d time.Duration) { time.Sleep(d) + w.mtx.RLock() + defer w.mtx.RUnlock() for q := range w.subscriptions { err := w.ws.Subscribe(context.Background(), q) if err != nil {