Skip to content

Commit

Permalink
fix: Revert "Improve BFT Tests Stability (#1385)" (#1441)
Browse files Browse the repository at this point in the history
This PR reverts #1385 following @jaekwon
#1320 (comment)

<!-- please provide a detailed description of the changes made in this
pull request. -->

<details><summary>Contributors' checklist...</summary>

- [ ] Added new tests, or not needed, or not feasible
- [ ] Provided an example (e.g. screenshot) to aid review or the PR is
self-explanatory
- [ ] Updated the official documentation or not needed
- [ ] No breaking changes were made, or a `BREAKING CHANGE: xxx` message
was included in the description
- [ ] Added references to related issues and PRs
- [ ] Provided any useful hints for running manual tests
- [ ] Added new benchmarks to [generated
graphs](https://gnoland.github.io/benchmarks), if any. More info
[here](https://github.com/gnolang/gno/blob/master/.benchmarks/README.md).
</details>

Signed-off-by: gfanton <[email protected]>
  • Loading branch information
gfanton authored Dec 17, 2023
1 parent ad9d30f commit 6cabad4
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 37 deletions.
38 changes: 1 addition & 37 deletions tm2/pkg/bft/consensus/common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
"os"
"path"
"path/filepath"
"reflect"
"sort"
"sync"
"testing"
Expand Down Expand Up @@ -246,50 +245,15 @@ func validatePrevoteAndPrecommit(t *testing.T, cs *ConsensusState, thisRound, lo
validatePrecommit(t, cs, thisRound, lockRound, privVal, votedBlockHash, lockedBlockHash)
}

func bufferedEventChannel(in <-chan events.Event, size int) (out chan events.Event) {
out = make(chan events.Event, size)
go func() {
defer close(out)
for evt := range in {
out <- evt
}
}()

return out
}

func subscribe(evsw events.EventSwitch, protoevent events.Event) <-chan events.Event {
name := reflect.ValueOf(protoevent).Type().Name()
listenerID := fmt.Sprintf("%s-%s", testSubscriber, name)
ch := events.SubscribeToEvent(evsw, listenerID, protoevent)

// Similar to subscribeToVoter, this modification introduces
// a buffered channel to ensures that events are consumed
// asynchronously, thereby avoiding the deadlock situation described in
// #1320 where the eventSwitch.FireEvent method was blocked.
bch := bufferedEventChannel(ch, 16)

return bch
}

func subscribeToVoter(cs *ConsensusState, addr crypto.Address) <-chan events.Event {
ch := events.SubscribeFiltered(cs.evsw, testSubscriber, func(event events.Event) bool {
return events.SubscribeFiltered(cs.evsw, testSubscriber, func(event events.Event) bool {
if vote, ok := event.(types.EventVote); ok {
if vote.Vote.ValidatorAddress == addr {
return true
}
}
return false
})

// This modification addresses the deadlock issue outlined in issue
// #1320. By creating a buffered channel, we ensure that events are
// consumed even if the main thread is blocked. This prevents the
// deadlock that occurred when eventSwitch.FireEvent was blocked due to
// no available consumers for the event.
bch := bufferedEventChannel(ch, 16)

return bch
}

// -------------------------------------------------------------------------------
Expand Down
5 changes: 5 additions & 0 deletions tm2/pkg/bft/consensus/state_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (

cstypes "github.com/gnolang/gno/tm2/pkg/bft/consensus/types"
"github.com/gnolang/gno/tm2/pkg/bft/types"
"github.com/gnolang/gno/tm2/pkg/events"
p2pmock "github.com/gnolang/gno/tm2/pkg/p2p/mock"
"github.com/gnolang/gno/tm2/pkg/random"
"github.com/gnolang/gno/tm2/pkg/testutils"
Expand Down Expand Up @@ -1777,3 +1778,7 @@ func TestStateOutputVoteStats(t *testing.T) {
case <-time.After(50 * time.Millisecond):
}
}

func subscribe(evsw events.EventSwitch, protoevent events.Event) <-chan events.Event {
return events.SubscribeToEvent(evsw, testSubscriber, protoevent)
}

0 comments on commit 6cabad4

Please sign in to comment.