From 6a0d9c832a400468f4884423f53e8630357b86b4 Mon Sep 17 00:00:00 2001 From: William Banfield <4561443+williambanfield@users.noreply.github.com> Date: Thu, 30 Sep 2021 12:19:18 -0400 Subject: [PATCH 1/3] blocksync: fix shutdown deadlock issue (#7030) When shutting down blocksync, it is observed that the process can hang completely. A dump of running goroutines reveals that this is due to goroutines not listening on the correct shutdown signal. Namely, the `poolRoutine` goroutine does not wait on `pool.Quit`. The `poolRoutine` does not receive any other shutdown signal during `OnStop` becuase it must stop before the `r.closeCh` is closed. Currently the `poolRoutine` listens in the `closeCh` which will not close until the `poolRoutine` stops and calls `poolWG.Done()`. This change also puts the `requestRoutine()` in the `OnStart` method to make it more visible since it does not rely on anything that is spawned in the `poolRoutine`. ``` goroutine 183 [semacquire]: sync.runtime_Semacquire(0xc0000d3bd8) runtime/sema.go:56 +0x45 sync.(*WaitGroup).Wait(0xc0000d3bd0) sync/waitgroup.go:130 +0x65 github.com/tendermint/tendermint/internal/blocksync/v0.(*Reactor).OnStop(0xc0000d3a00) github.com/tendermint/tendermint/internal/blocksync/v0/reactor.go:193 +0x47 github.com/tendermint/tendermint/libs/service.(*BaseService).Stop(0xc0000d3a00, 0x0, 0x0) github.com/tendermint/tendermint/libs/service/service.go:171 +0x323 github.com/tendermint/tendermint/node.(*nodeImpl).OnStop(0xc00052c000) github.com/tendermint/tendermint/node/node.go:758 +0xc62 github.com/tendermint/tendermint/libs/service.(*BaseService).Stop(0xc00052c000, 0x0, 0x0) github.com/tendermint/tendermint/libs/service/service.go:171 +0x323 github.com/tendermint/tendermint/cmd/tendermint/commands.NewRunNodeCmd.func1.1() github.com/tendermint/tendermint/cmd/tendermint/commands/run_node.go:143 +0x62 github.com/tendermint/tendermint/libs/os.TrapSignal.func1(0xc000df6d20, 0x7f04a68da900, 0xc0004a8930, 0xc0005a72d8) github.com/tendermint/tendermint/libs/os/os.go:26 +0x102 created by github.com/tendermint/tendermint/libs/os.TrapSignal github.com/tendermint/tendermint/libs/os/os.go:22 +0xe6 goroutine 161 [select]: github.com/tendermint/tendermint/internal/blocksync/v0.(*Reactor).poolRoutine(0xc0000d3a00, 0x0) github.com/tendermint/tendermint/internal/blocksync/v0/reactor.go:464 +0x2b3 created by github.com/tendermint/tendermint/internal/blocksync/v0.(*Reactor).OnStart github.com/tendermint/tendermint/internal/blocksync/v0/reactor.go:174 +0xf1 goroutine 162 [select]: github.com/tendermint/tendermint/internal/blocksync/v0.(*Reactor).processBlockSyncCh(0xc0000d3a00) github.com/tendermint/tendermint/internal/blocksync/v0/reactor.go:310 +0x151 created by github.com/tendermint/tendermint/internal/blocksync/v0.(*Reactor).OnStart github.com/tendermint/tendermint/internal/blocksync/v0/reactor.go:177 +0x54 goroutine 163 [select]: github.com/tendermint/tendermint/internal/blocksync/v0.(*Reactor).processPeerUpdates(0xc0000d3a00) github.com/tendermint/tendermint/internal/blocksync/v0/reactor.go:363 +0x12b created by github.com/tendermint/tendermint/internal/blocksync/v0.(*Reactor).OnStart github.com/tendermint/tendermint/internal/blocksync/v0/reactor.go:178 +0x76 ``` --- internal/blocksync/v0/reactor.go | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/internal/blocksync/v0/reactor.go b/internal/blocksync/v0/reactor.go index 286df0ef0..552dcbda5 100644 --- a/internal/blocksync/v0/reactor.go +++ b/internal/blocksync/v0/reactor.go @@ -169,6 +169,8 @@ func (r *Reactor) OnStart() error { if err := r.pool.Start(); err != nil { return err } + r.poolWG.Add(1) + go r.requestRoutine() r.poolWG.Add(1) go r.poolRoutine(false) @@ -384,6 +386,9 @@ func (r *Reactor) SwitchToBlockSync(state sm.State) error { r.syncStartTime = time.Now() + r.poolWG.Add(1) + go r.requestRoutine() + r.poolWG.Add(1) go r.poolRoutine(true) @@ -394,7 +399,6 @@ func (r *Reactor) requestRoutine() { statusUpdateTicker := time.NewTicker(statusUpdateIntervalSeconds * time.Second) defer statusUpdateTicker.Stop() - r.poolWG.Add(1) defer r.poolWG.Done() for { @@ -455,8 +459,6 @@ func (r *Reactor) poolRoutine(stateSynced bool) { defer trySyncTicker.Stop() defer switchToConsensusTicker.Stop() - go r.requestRoutine() - defer r.poolWG.Done() FOR_LOOP: @@ -605,6 +607,8 @@ FOR_LOOP: case <-r.closeCh: break FOR_LOOP + case <-r.pool.Quit(): + break FOR_LOOP } } } From 77052370cce980fa378d7acc414120faf80ab573 Mon Sep 17 00:00:00 2001 From: "M. J. Fromberger" Date: Thu, 30 Sep 2021 10:13:32 -0700 Subject: [PATCH 2/3] Update default config template to match mapstructure keys. (#7036) Fix a couple of cases where we updated the keys in the config reader, but forgot to update some of their uses in the default template. Fixes #7031. --- config/toml.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/config/toml.go b/config/toml.go index 986ff1c2a..e4c86c9f4 100644 --- a/config/toml.go +++ b/config/toml.go @@ -164,10 +164,10 @@ laddr = "{{ .PrivValidator.ListenAddr }}" client-certificate-file = "{{ js .PrivValidator.ClientCertificate }}" # Client key generated while creating certificates for secure connection -validator-client-key-file = "{{ js .PrivValidator.ClientKey }}" +client-key-file = "{{ js .PrivValidator.ClientKey }}" # Path to the Root Certificate Authority used to sign both client and server certificates -certificate-authority = "{{ js .PrivValidator.RootCA }}" +root-ca-file = "{{ js .PrivValidator.RootCA }}" ####################################################################### @@ -447,8 +447,8 @@ rpc-servers = "{{ StringsJoin .StateSync.RPCServers "," }}" trust-height = {{ .StateSync.TrustHeight }} trust-hash = "{{ .StateSync.TrustHash }}" -# The trust period should be set so that Tendermint can detect and gossip misbehavior before -# it is considered expired. For chains based on the Cosmos SDK, one day less than the unbonding +# The trust period should be set so that Tendermint can detect and gossip misbehavior before +# it is considered expired. For chains based on the Cosmos SDK, one day less than the unbonding # period should suffice. trust-period = "{{ .StateSync.TrustPeriod }}" From bdd815ebc9209e11947353a12557558e81f4e5d5 Mon Sep 17 00:00:00 2001 From: "M. J. Fromberger" Date: Thu, 30 Sep 2021 10:53:05 -0700 Subject: [PATCH 3/3] Align atomic struct field for compatibility in 32-bit ABIs. (#7037) The layout of struct fields means that interior fields may not be properly aligned for 64-bit access. Fixes #7000. --- internal/p2p/conn/connection.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/internal/p2p/conn/connection.go b/internal/p2p/conn/connection.go index dc5bacc39..339ad7469 100644 --- a/internal/p2p/conn/connection.go +++ b/internal/p2p/conn/connection.go @@ -751,13 +751,18 @@ func (chDesc ChannelDescriptor) FillDefaults() (filled ChannelDescriptor) { // TODO: lowercase. // NOTE: not goroutine-safe. type Channel struct { + // Exponential moving average. + // This field must be accessed atomically. + // It is first in the struct to ensure correct alignment. + // See https://github.com/tendermint/tendermint/issues/7000. + recentlySent int64 + conn *MConnection desc ChannelDescriptor sendQueue chan []byte sendQueueSize int32 // atomic. recving []byte sending []byte - recentlySent int64 // exponential moving average maxPacketMsgPayloadSize int