abci: localClient improvements & bugfixes & pubsub Unsubscribe issues #2748

melekes · 2018-11-02T10:51:09Z

~~Updated all relevant documentation in docs~~
Updated all code comments where relevant
~~Wrote tests~~
Updated CHANGELOG_PENDING.md

codecov-io · 2018-11-02T13:59:47Z

Codecov Report

Merging #2748 into develop will decrease coverage by 0.1%.
The diff coverage is 26.19%.

@@             Coverage Diff             @@
##           develop    #2748      +/-   ##
===========================================
- Coverage    62.29%   62.19%   -0.11%     
===========================================
  Files          212      212              
  Lines        17219    17253      +34     
===========================================
+ Hits         10727    10730       +3     
- Misses        5591     5619      +28     
- Partials       901      904       +3

Impacted Files	Coverage Δ
libs/pubsub/pubsub.go	`83.89% <ø> (ø)`	⬆️
mempool/mempool.go	`77.81% <ø> (ø)`	⬆️
rpc/core/consensus.go	`0% <0%> (ø)`	⬆️
rpc/core/mempool.go	`0% <0%> (ø)`	⬆️
evidence/reactor.go	`62.62% <0%> (-2.38%)`	⬇️
consensus/replay_file.go	`0% <0%> (ø)`	⬆️
p2p/pex/addrbook.go	`68.91% <100%> (ø)`	⬆️
mempool/reactor.go	`64.19% <14.28%> (ø)`	⬆️
consensus/reactor.go	`65.93% <36.36%> (-1.05%)`	⬇️
consensus/state.go	`79.78% <60%> (ø)`	⬆️
... and 3 more

Refs #2721

``` [54310]: E[11-02|11:59:39.851] Connection failed @ sendRoutine module=p2p peer=0xb78f00 conn=MConn{74.207.236.148:26656} err="pong timeout" ``` #2721 (comment)

#2721 (comment) It's confusing that sometimes we check if peer has a state, but most of the times we expect it to be there 1. https://github.com/tendermint/tendermint/blob/add79700b5fe84417538202b6c927c8cc5383672/mempool/reactor.go#L138 2. https://github.com/tendermint/tendermint/blob/add79700b5fe84417538202b6c927c8cc5383672/rpc/core/consensus.go#L196 (edited) I will change everything to always assume peer has a state and panic otherwise that should help identify issues earlier

App callback should be protected by lock as well (note this was already done for InitChainAsync, why not for others???). Otherwise, when we execute the block, tx might come in and call the callback in the same time we're updating it in execBlockOnProxyApp => DATA RACE Fixes #2721 Consensus state is locked ``` goroutine 113333 [semacquire, 309 minutes]: sync.runtime_SemacquireMutex(0xc00180009c, 0xc0000c7e00) /usr/local/go/src/runtime/sema.go:71 +0x3d sync.(*RWMutex).RLock(0xc001800090) /usr/local/go/src/sync/rwmutex.go:50 +0x4e github.com/MinterTeam/minter-go-node/vendor/github.com/tendermint/tendermint/consensus.(*ConsensusState).GetRoundState(0xc001800000, 0x0) /root/go/src/github.com/MinterTeam/minter-go-node/vendor/github.com/tendermint/tendermint/consensus/state.go:218 +0x46 github.com/MinterTeam/minter-go-node/vendor/github.com/tendermint/tendermint/consensus.(*ConsensusReactor).queryMaj23Routine(0xc0017def80, 0x11104a0, 0xc0072488f0, 0xc007248 9c0) /root/go/src/github.com/MinterTeam/minter-go-node/vendor/github.com/tendermint/tendermint/consensus/reactor.go:735 +0x16d created by github.com/MinterTeam/minter-go-node/vendor/github.com/tendermint/tendermint/consensus.(*ConsensusReactor).AddPeer /root/go/src/github.com/MinterTeam/minter-go-node/vendor/github.com/tendermint/tendermint/consensus/reactor.go:172 +0x236 ``` because localClient is locked ``` goroutine 1899 [semacquire, 309 minutes]: sync.runtime_SemacquireMutex(0xc00003363c, 0xc0000cb500) /usr/local/go/src/runtime/sema.go:71 +0x3d sync.(*Mutex).Lock(0xc000033638) /usr/local/go/src/sync/mutex.go:134 +0xff github.com/MinterTeam/minter-go-node/vendor/github.com/tendermint/tendermint/abci/client.(*localClient).SetResponseCallback(0xc0001fb560, 0xc007868540) /root/go/src/github.com/MinterTeam/minter-go-node/vendor/github.com/tendermint/tendermint/abci/client/local_client.go:32 +0x33 github.com/MinterTeam/minter-go-node/vendor/github.com/tendermint/tendermint/proxy.(*appConnConsensus).SetResponseCallback(0xc00002f750, 0xc007868540) /root/go/src/github.com/MinterTeam/minter-go-node/vendor/github.com/tendermint/tendermint/proxy/app_conn.go:57 +0x40 github.com/MinterTeam/minter-go-node/vendor/github.com/tendermint/tendermint/state.execBlockOnProxyApp(0x1104e20, 0xc002ca0ba0, 0x11092a0, 0xc00002f750, 0xc0001fe960, 0xc000bfc660, 0x110cfe0, 0xc000090330, 0xc9d12, 0xc000d9d5a0, ...) /root/go/src/github.com/MinterTeam/minter-go-node/vendor/github.com/tendermint/tendermint/state/execution.go:230 +0x1fd github.com/MinterTeam/minter-go-node/vendor/github.com/tendermint/tendermint/state.(*BlockExecutor).ApplyBlock(0xc002c2a230, 0x7, 0x0, 0xc000eae880, 0x6, 0xc002e52c60, 0x16, 0x1f927, 0xc9d12, 0xc000d9d5a0, ...) /root/go/src/github.com/MinterTeam/minter-go-node/vendor/github.com/tendermint/tendermint/state/execution.go:96 +0x142 github.com/MinterTeam/minter-go-node/vendor/github.com/tendermint/tendermint/consensus.(*ConsensusState).finalizeCommit(0xc001800000, 0x1f928) /root/go/src/github.com/MinterTeam/minter-go-node/vendor/github.com/tendermint/tendermint/consensus/state.go:1339 +0xa3e github.com/MinterTeam/minter-go-node/vendor/github.com/tendermint/tendermint/consensus.(*ConsensusState).tryFinalizeCommit(0xc001800000, 0x1f928) /root/go/src/github.com/MinterTeam/minter-go-node/vendor/github.com/tendermint/tendermint/consensus/state.go:1270 +0x451 github.com/MinterTeam/minter-go-node/vendor/github.com/tendermint/tendermint/consensus.(*ConsensusState).enterCommit.func1(0xc001800000, 0x0, 0x1f928) /root/go/src/github.com/MinterTeam/minter-go-node/vendor/github.com/tendermint/tendermint/consensus/state.go:1218 +0x90 github.com/MinterTeam/minter-go-node/vendor/github.com/tendermint/tendermint/consensus.(*ConsensusState).enterCommit(0xc001800000, 0x1f928, 0x0) /root/go/src/github.com/MinterTeam/minter-go-node/vendor/github.com/tendermint/tendermint/consensus/state.go:1247 +0x6b8 github.com/MinterTeam/minter-go-node/vendor/github.com/tendermint/tendermint/consensus.(*ConsensusState).addVote(0xc001800000, 0xc003d8dea0, 0xc000cf4cc0, 0x28, 0xf1, 0xc003bc7ad0, 0xc003bc7b10) /root/go/src/github.com/MinterTeam/minter-go-node/vendor/github.com/tendermint/tendermint/consensus/state.go:1659 +0xbad github.com/MinterTeam/minter-go-node/vendor/github.com/tendermint/tendermint/consensus.(*ConsensusState).tryAddVote(0xc001800000, 0xc003d8dea0, 0xc000cf4cc0, 0x28, 0xf1, 0xf1, 0xf1) /root/go/src/github.com/MinterTeam/minter-go-node/vendor/github.com/tendermint/tendermint/consensus/state.go:1517 +0x59 github.com/MinterTeam/minter-go-node/vendor/github.com/tendermint/tendermint/consensus.(*ConsensusState).handleMsg(0xc001800000, 0xd98200, 0xc0070dbed0, 0xc000cf4cc0, 0x28) /root/go/src/github.com/MinterTeam/minter-go-node/vendor/github.com/tendermint/tendermint/consensus/state.go:660 +0x64b github.com/MinterTeam/minter-go-node/vendor/github.com/tendermint/tendermint/consensus.(*ConsensusState).receiveRoutine(0xc001800000, 0x0) /root/go/src/github.com/MinterTeam/minter-go-node/vendor/github.com/tendermint/tendermint/consensus/state.go:617 +0x670 created by github.com/MinterTeam/minter-go-node/vendor/github.com/tendermint/tendermint/consensus.(*ConsensusState).OnStart /root/go/src/github.com/MinterTeam/minter-go-node/vendor/github.com/tendermint/tendermint/consensus/state.go:311 +0x132 ``` tx comes in and CheckTx is executed right when we execute the block ``` goroutine 111044 [semacquire, 309 minutes]: sync.runtime_SemacquireMutex(0xc00003363c, 0x0) /usr/local/go/src/runtime/sema.go:71 +0x3d sync.(*Mutex).Lock(0xc000033638) /usr/local/go/src/sync/mutex.go:134 +0xff github.com/MinterTeam/minter-go-node/vendor/github.com/tendermint/tendermint/abci/client.(*localClient).CheckTxAsync(0xc0001fb0e0, 0xc002d94500, 0x13f, 0x280, 0x0) /root/go/src/github.com/MinterTeam/minter-go-node/vendor/github.com/tendermint/tendermint/abci/client/local_client.go:85 +0x47 github.com/MinterTeam/minter-go-node/vendor/github.com/tendermint/tendermint/proxy.(*appConnMempool).CheckTxAsync(0xc00002f720, 0xc002d94500, 0x13f, 0x280, 0x1) /root/go/src/github.com/MinterTeam/minter-go-node/vendor/github.com/tendermint/tendermint/proxy/app_conn.go:114 +0x51 github.com/MinterTeam/minter-go-node/vendor/github.com/tendermint/tendermint/mempool.(*Mempool).CheckTx(0xc002d3a320, 0xc002d94500, 0x13f, 0x280, 0xc0072355f0, 0x0, 0x0) /root/go/src/github.com/MinterTeam/minter-go-node/vendor/github.com/tendermint/tendermint/mempool/mempool.go:316 +0x17b github.com/MinterTeam/minter-go-node/vendor/github.com/tendermint/tendermint/rpc/core.BroadcastTxSync(0xc002d94500, 0x13f, 0x280, 0x0, 0x0, 0x0) /root/go/src/github.com/MinterTeam/minter-go-node/vendor/github.com/tendermint/tendermint/rpc/core/mempool.go:93 +0xb8 reflect.Value.call(0xd85560, 0x10326c0, 0x13, 0xec7b8b, 0x4, 0xc00663f180, 0x1, 0x1, 0xc00663f180, 0xc00663f188, ...) /usr/local/go/src/reflect/value.go:447 +0x449 reflect.Value.Call(0xd85560, 0x10326c0, 0x13, 0xc00663f180, 0x1, 0x1, 0x0, 0x0, 0xc005cc9344) /usr/local/go/src/reflect/value.go:308 +0xa4 github.com/MinterTeam/minter-go-node/vendor/github.com/tendermint/tendermint/rpc/lib/server.makeHTTPHandler.func2(0x1102060, 0xc00663f100, 0xc0082d7900) /root/go/src/github.com/MinterTeam/minter-go-node/vendor/github.com/tendermint/tendermint/rpc/lib/server/handlers.go:269 +0x188 net/http.HandlerFunc.ServeHTTP(0xc002c81f20, 0x1102060, 0xc00663f100, 0xc0082d7900) /usr/local/go/src/net/http/server.go:1964 +0x44 net/http.(*ServeMux).ServeHTTP(0xc002c81b60, 0x1102060, 0xc00663f100, 0xc0082d7900) /usr/local/go/src/net/http/server.go:2361 +0x127 github.com/MinterTeam/minter-go-node/vendor/github.com/tendermint/tendermint/rpc/lib/server.maxBytesHandler.ServeHTTP(0x10f8a40, 0xc002c81b60, 0xf4240, 0x1102060, 0xc00663f100, 0xc0082d7900) /root/go/src/github.com/MinterTeam/minter-go-node/vendor/github.com/tendermint/tendermint/rpc/lib/server/http_server.go:219 +0xcf github.com/MinterTeam/minter-go-node/vendor/github.com/tendermint/tendermint/rpc/lib/server.RecoverAndLogHandler.func1(0x1103220, 0xc00121e620, 0xc0082d7900) /root/go/src/github.com/MinterTeam/minter-go-node/vendor/github.com/tendermint/tendermint/rpc/lib/server/http_server.go:192 +0x394 net/http.HandlerFunc.ServeHTTP(0xc002c06ea0, 0x1103220, 0xc00121e620, 0xc0082d7900) /usr/local/go/src/net/http/server.go:1964 +0x44 net/http.serverHandler.ServeHTTP(0xc001a1aa90, 0x1103220, 0xc00121e620, 0xc0082d7900) /usr/local/go/src/net/http/server.go:2741 +0xab net/http.(*conn).serve(0xc00785a3c0, 0x11041a0, 0xc000f844c0) /usr/local/go/src/net/http/server.go:1847 +0x646 created by net/http.(*Server).Serve /usr/local/go/src/net/http/server.go:2851 +0x2f5 ```

Read https://github.com/tendermint/tendermint/blob/55362ed76630f3e1ebec159a598f6a9fb5892cb1/libs/pubsub/pubsub.go#L13 for the detailed explanation of the issue. We'll need to fix it someday. Make sure to keep an eye on https://github.com/tendermint/tendermint/blob/master/docs/architecture/adr-033-pubsub.md

…consensus in /dump_consensus_state RPC endpoint, skip a peer with no state

also, do not log DeliverTx result (to be consistent with other memthods)

ebuchman

If I understand correctly this PR is addressing multiple distinct issues:

If the app panics, we deadlock. The app panicing is undefined behaviour, but as discussed previously it would probably be ideal for rpc endpoints to still be useful in this scenario - they shouldn't block, and it should be easy to figure out there's a problem like the app panicing
PeerState being accessed in the mempool reactor before it's set in the consensus reactor. This can happen if we have txs to send to a peer right away and mempool routine is ready to do that before ConsensusReactor.AddPeer has finished running (non-deterministic due to the order of reactor.AddPeer being iteration over a map)
Under heavy load, the BroadcastTxCommit endpoint saturates the pubsub and eventually deadlocks as subscription channels are sometimes not fully drained. Is it true that this only happens when CheckTx fails, and so the method returns before we get to select over deliverTxrResCh ?

Really great work getting to the bottom of all of this - thanks a lot!

abci/client/client.go

ebuchman · 2018-11-11T16:27:49Z

abci/client/local_client.go

 		types.ToRequestInitChain(req),
 		types.ToResponseInitChain(res),
 	)
-	app.mtx.Unlock()


yikes, an outisder!

ebuchman · 2018-11-11T16:29:40Z

abci/client/local_client.go

@@ -154,64 +170,73 @@ func (app *localClient) EchoSync(msg string) (*types.ResponseEcho, error) {

 func (app *localClient) InfoSync(req types.RequestInfo) (*types.ResponseInfo, error) {
 	app.mtx.Lock()
+	defer app.mtx.Unlock()


I feel like we can keep this without the defer in such small functions, no ?

Not if we want to be more or less resilient to app failures. If there's no app call, then sure.

ebuchman · 2018-11-11T16:31:14Z

consensus/reactor.go

-	ps := src.Get(types.PeerStateKey).(*PeerState)
+	ps, ok := src.Get(types.PeerStateKey).(*PeerState)
+	if !ok {
+		panic(fmt.Sprintf("Peer %v has no state", src))


What is the benefit of checking ok if we're just going to panic anyways?

?? we only panic if src has no state (!ok). I don't understand what do you mean by "anyways"

My understanding is we only get !ok if ps := src.Get(types.PeerStateKey).(*PeerState) (ie. without the ok) would have paniced. The point of the ok is to prevent the native panic and do something else. But here we're just explicilty panicing - so why even bother with the ok?

ah, right. probably not much sense except better error message

ebuchman · 2018-11-11T16:33:29Z

evidence/reactor.go

-	if !ok {
-		evR.Logger.Info("Found peer without PeerState", "peer", peer)
+	if !ok { 
+		// Peer does not have a state yet. We set it in the consensus reactor, but


This is kind of gross. Wish we had something better to co-ordinate this.

ebuchman · 2018-11-11T16:40:47Z

p2p/pex/addrbook.go

 	a.ourAddrs[addr.String()] = struct{}{}
+	a.mtx.Unlock()


I feel like if the function is more than one line, and especially if its not on a hot path, we should just use defer

I disagree. I think we should only use defer & mutexes when a) there're multiple return conditions b) we call other functions while locked and they (func) can panic c) objects protected by lock accessed throughout all func body.

https://twitter.com/akaliaev/status/1058384359844204544

Ah OK, this answers my 1st question, too.

I guess my concern is that the function might change over time, new return statements be added, and the Unlock will be forgotten about. If we use defer, adding new returns will not be a problem.

addr.String() is a function that can panic (if not now, in the future), so we should be using defer here as well.
It's too easy to forget to unlock, or to mutate a line to call a function, which may panic now or in the future...
So I'd prefer that we just use defers whenever possible.

addr.String() is a function that can panic

fair point

ebuchman · 2018-11-11T16:44:29Z

rpc/core/mempool.go

 	ctx, cancel := context.WithTimeout(context.Background(), subscribeTimeout)
 	defer cancel()
-	deliverTxResCh := make(chan interface{})
+	deliverTxResCh := make(chan interface{}, 1)


Why does it need the buffer?

because a) we don't want to lock the synchronous pubsub b) we drain it if no result or any error (see defer below; you can't drain non-buffered channel)

ebuchman · 2018-11-11T16:46:50Z

rpc/core/mempool.go

-	// TODO: configurable?
-	timer := time.NewTimer(60 * 2 * time.Second)
+	// Wait for the tx to be included in a block or timeout.
+	var deliverTxTimeout = 10 * time.Second // TODO: configurable?


This might be too low. We should open a new issue about exposing this as a parameter. I think we would want a default set in the config.toml, but also maybe a url parameter so it can be overridden (within reason) in the http request (?).

120 sec. was too much. But I agree, it should be configurable.

also maybe a url parameter so it can be overridden (within reason) in the http request

this is an attack vector

this is an attack vector

Hence within reason. We can put a reasonable limit on what that value can be, and have a low default. Not sure if it's really a good idea though.

ebuchman · 2018-11-12T13:24:06Z

@melekes can you answer

Under heavy load, the BroadcastTxCommit endpoint saturates the pubsub and eventually deadlocks as subscription channels are sometimes not fully drained. Is it true that this only happens when CheckTx fails, and so the method returns before we get to select over deliverTxrResCh ?

melekes · 2018-11-13T11:37:37Z

Is it true that this only happens when CheckTx fails, and so the method returns before we get to select over deliverTxrResCh ?

Yes

liamsi · 2018-11-13T13:03:38Z

abci/client/local_client.go

 	app.Callback = cb
+	app.mtx.Unlock()


~~Is this not deferred because the line above simply can't panic?~~ NVM

liamsi · 2018-11-13T13:14:43Z

consensus/reactor.go

@@ -293,9 +300,9 @@ func (conR *ConsensusReactor) Receive(chID byte, src p2p.Peer, msgBytes []byte)
 		switch msg := msg.(type) {
 		case *VoteMessage:
 			cs := conR.conS
-			cs.mtx.Lock()
+			cs.mtx.RLock()


ebuchman · 2018-11-13T15:13:19Z

CHANGELOG_PENDING.md

@@ -26,3 +26,6 @@ Friendly reminder, we have a [bug bounty program](https://hackerone.com/tendermi

 ### BUG FIXES:

+- [abci] unlock mutex in localClient so even when app panics (e.g. during CheckTx), consensus continue working


I'm over due to write a proper description of how to do changelogs (and ultimately a linter!) but this should include:

capitalize the start of the entry

include PR number

ebuchman · 2018-11-13T15:17:33Z

abci/client/local_client.go

@@ -53,8 +61,9 @@ func (app *localClient) EchoAsync(msg string) *ReqRes {

 func (app *localClient) InfoAsync(req types.RequestInfo) *ReqRes {
 	app.mtx.Lock()
+	defer app.mtx.Unlock()


I get that we need defer to catch app panics, but are we sure we want the lock to be held during the callback?

ebuchman · 2018-11-13T15:26:12Z

p2p/pex/addrbook.go

 	a.ourAddrs[addr.String()] = struct{}{}
+	a.mtx.Unlock()


I guess my concern is that the function might change over time, new return statements be added, and the Unlock will be forgotten about. If we use defer, adding new returns will not be a problem.

jaekwon · 2018-12-16T02:05:44Z

p2p/pex/addrbook.go

@@ -178,10 +178,10 @@ func (a *addrBook) OurAddress(addr *p2p.NetAddress) bool {

 func (a *addrBook) AddPrivateIDs(IDs []string) {
 	a.mtx.Lock()
-	defer a.mtx.Unlock()
 	for _, id := range IDs {
 		a.privateIDs[p2p.ID(id)] = struct{}{}


jaekwon · 2018-12-16T02:06:04Z

p2p/pex/addrbook.go

 	addrs := []*knownAddress{}
+	a.mtx.Lock()
 	for _, addr := range a.addrLookup {
 		addrs = append(addrs, addr.copy())


jaekwon · 2018-12-16T12:36:43Z

rpc/core/mempool.go

-	defer eventBus.Unsubscribe(context.Background(), "mempool", q)
+	defer func() {
+		// drain deliverTxResCh to make sure we don't block
+	LOOP:


Don't you want to drain after Unsubscribe, thus guaranteeing that no further items can be sent to the channel? In between draining here and Unsubscribe, another item may have been sent to the channel. (which is actually fine as long as only 1 item is ever sent to the channel because the capacity is 1, and such channels (even those that have items pending) get garbage collected if there is no receiver anymore).

Because unused channels are supposed to be garbage collected anyways, draining is often not the right thing to do.

#2748 (comment)

melekes requested review from ebuchman, xla and zramsay as code owners November 2, 2018 10:51

melekes changed the base branch from master to develop November 2, 2018 10:51

melekes mentioned this pull request Nov 2, 2018

Tendermint became unresponsive after some load #2721

Closed

melekes changed the title ~~[WIP] Unresponsive Tendermint~~ abci: localClient improvements & bugfixes Nov 8, 2018

melekes added 9 commits November 8, 2018 12:42

use READ lock/unlock in ConsensusState#GetLastHeight

3c96d30

Refs #2721

do not use defers when there's no need

9d483e8

fix peer formatting (output its address instead of the pointer)

07d091f

``` [54310]: E[11-02|11:59:39.851] Connection failed @ sendRoutine module=p2p peer=0xb78f00 conn=MConn{74.207.236.148:26656} err="pong timeout" ``` #2721 (comment)

consensus: use read lock in Receive#VoteMessage

5b6fdfb

use defer to unlock mutex because application might panic

5a46880

use defer in every method of the localClient

977a7d9

add a changelog entry

aec480c

melekes force-pushed the 2721-unresponsive-tm branch 2 times, most recently from ee43715 to 611268a Compare November 8, 2018 14:42

melekes changed the title ~~abci: localClient improvements & bugfixes~~ abci: localClient improvements & bugfixes & pubsub Unsubscribe issues Nov 8, 2018

melekes force-pushed the 2721-unresponsive-tm branch from 611268a to ab6cebb Compare November 8, 2018 15:03

melekes force-pushed the 2721-unresponsive-tm branch from ab6cebb to f4f3109 Compare November 8, 2018 15:25

melekes added 3 commits November 9, 2018 13:01

retry instead of panic when peer has no state in reactors other than …

223b069

…consensus in /dump_consensus_state RPC endpoint, skip a peer with no state

rpc/core/mempool: simplify error messages

2687c48

rpc/core/mempool: use time.After instead of timer

8db23b0

also, do not log DeliverTx result (to be consistent with other memthods)

ebuchman reviewed Nov 11, 2018

View reviewed changes

Merge branch 'develop' into 2721-unresponsive-tm

e936961

unlock before calling the callback in reqRes#SetCallback

439cb1c

melekes mentioned this pull request Nov 13, 2018

rpc: too many broadcast_commit calls can lead to consensus liveness issues #2826

Closed

liamsi reviewed Nov 13, 2018

View reviewed changes

ebuchman approved these changes Nov 13, 2018

View reviewed changes

ebuchman merged commit 5a6822c into develop Nov 13, 2018

ebuchman deleted the 2721-unresponsive-tm branch November 13, 2018 16:32

ebuchman mentioned this pull request Nov 17, 2018

We should still process RPC requests even if the consensus failed #1772

Closed

jaekwon reviewed Dec 16, 2018

View reviewed changes

melekes mentioned this pull request Dec 17, 2018

add design philosophy doc #3034

Merged

zemyblue mentioned this pull request May 29, 2020

Lock contention for access to consensus state between rpc request and tx processing occurs Finschia/ostracon#82

Closed

4 tasks

jinsankim mentioned this pull request Sep 28, 2020

Remove localClient.mtx #5411

Closed

4 tasks

melekes added a commit that referenced this pull request Feb 23, 2021

abci/client/local_client: do not hold mtx during callback

d4c4eab

#2748 (comment)

melekes mentioned this pull request Feb 23, 2021

abci/client/local_client: do not hold mtx during callback #6170

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

abci: localClient improvements & bugfixes & pubsub Unsubscribe issues #2748

abci: localClient improvements & bugfixes & pubsub Unsubscribe issues #2748

melekes commented Nov 2, 2018 •

edited

codecov-io commented Nov 2, 2018 •

edited

ebuchman left a comment •

edited

ebuchman Nov 11, 2018

ebuchman Nov 11, 2018

melekes Nov 12, 2018

ebuchman Nov 11, 2018

melekes Nov 12, 2018

ebuchman Nov 12, 2018 •

edited

melekes Nov 12, 2018

ebuchman Nov 11, 2018

ebuchman Nov 11, 2018

melekes Nov 12, 2018

liamsi Nov 13, 2018

ebuchman Nov 13, 2018

jaekwon Dec 16, 2018 •

edited

melekes Dec 16, 2018

ebuchman Nov 11, 2018

melekes Nov 12, 2018

ebuchman Nov 11, 2018

melekes Nov 12, 2018

ebuchman Nov 12, 2018

ebuchman commented Nov 12, 2018

melekes commented Nov 13, 2018

liamsi Nov 13, 2018 •

edited

liamsi Nov 13, 2018

ebuchman Nov 13, 2018

ebuchman Nov 13, 2018

ebuchman Nov 13, 2018

jaekwon Dec 16, 2018

jaekwon Dec 16, 2018

jaekwon Dec 16, 2018

		@@ -26,3 +26,6 @@ Friendly reminder, we have a [bug bounty program](https://hackerone.com/tendermi

		### BUG FIXES:

		- [abci] unlock mutex in localClient so even when app panics (e.g. during CheckTx), consensus continue working

abci: localClient improvements & bugfixes & pubsub Unsubscribe issues #2748

abci: localClient improvements & bugfixes & pubsub Unsubscribe issues #2748

Conversation

melekes commented Nov 2, 2018 • edited

codecov-io commented Nov 2, 2018 • edited

Codecov Report

ebuchman left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ebuchman Nov 12, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jaekwon Dec 16, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ebuchman commented Nov 12, 2018

melekes commented Nov 13, 2018

liamsi Nov 13, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

melekes commented Nov 2, 2018 •

edited

codecov-io commented Nov 2, 2018 •

edited

ebuchman left a comment •

edited

ebuchman Nov 12, 2018 •

edited

jaekwon Dec 16, 2018 •

edited

liamsi Nov 13, 2018 •

edited