Skip to content

fix: potential deadlock in pex module tests#594

Merged
shotonoff merged 1 commit intov0.10-devfrom
td-147-fix-potential-deadlock
Feb 27, 2023
Merged

fix: potential deadlock in pex module tests#594
shotonoff merged 1 commit intov0.10-devfrom
td-147-fix-potential-deadlock

Conversation

@shotonoff
Copy link
Collaborator

@shotonoff shotonoff commented Feb 23, 2023

Issue being fixed or feature implemented

The tests in pex module are flaky, i.e. TestReactorConnectFullNetwork

go test ./internal/p2p/pex -test.run TestReactorConnectFullNetwork -race -test.v -test.count 10
=== RUN   TestReactorConnectFullNetwork
    reactor_test.go:65: started ef59301921b33674348cc04ef4e63f8159bdb234
    reactor_test.go:65: started 395bc84197103724210f39587cb11a597c631696
    reactor_test.go:65: started 47c54b7311f120d81702f23b399761985df35fa4
    reactor_test.go:65: started 436ed2bf9bac168ca7f403c5f78c66e7fabc94d3
--- PASS: TestReactorConnectFullNetwork (2.02s)
=== RUN   TestReactorConnectFullNetwork
    reactor_test.go:65: started 21ac0dd98f38a817e15389961511803c164dfbb1
    reactor_test.go:65: started afaff974a583bcb46237ff53a1fc9d8f423b33d4
    reactor_test.go:65: started 37f264190bb20b4fb67c923f07f0818256262a87
    reactor_test.go:65: started d8e026d4a59747158fb3d03e1dbbc90f42943a49
POTENTIAL DEADLOCK:
Previous place where the lock was grabbed
goroutine 1663 lock 0xc0000ef778
../peermanager.go:851 p2p.(*PeerManager).Ready { m.mtx.Lock() } <<<<<
../peermanager.go:850 p2p.(*PeerManager).Ready { func (m *PeerManager) Ready(ctx context.Context, peerID types.NodeID, channels ChannelIDSet) { }
../router.go:722 p2p.(*Router).routePeer {  }

Have been trying to lock it again for more than 30s
goroutine 1267 lock 0xc0000ef778
../peermanager.go:581 p2p.(*PeerManager).TryDialNext { m.mtx.Lock() } <<<<<
../peermanager.go:580 p2p.(*PeerManager).TryDialNext { func (m *PeerManager) TryDialNext() NodeAddress { }
../peermanager.go:558 p2p.(*PeerManager).DialNext { for counter := uint32(0); ; counter++ { }
../router.go:557 p2p.(*Router).dialPeers { for { }

What was done?

Added ability to remove a subscription from the list through PeerManager.Unsubscribe

How Has This Been Tested?

Unit / E2E tests

Breaking Changes

N/A

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have made corresponding changes to the documentation

For repository code-owners and collaborators only

  • I have assigned this pull request to a milestone

@shotonoff shotonoff merged commit 6ece199 into v0.10-dev Feb 27, 2023
@shotonoff shotonoff deleted the td-147-fix-potential-deadlock branch February 27, 2023 10:13
shotonoff added a commit that referenced this pull request Mar 1, 2023
* fix: add PeerManager.Unsubscribe method (#594)

* build(deps): Bump actions/checkout from 2.3.4 to 3.3.0 (#595)

Bumps [actions/checkout](https://github.com/actions/checkout) from 2.3.4 to 3.3.0.
- [Release notes](https://github.com/actions/checkout/releases)
- [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md)
- [Commits](actions/checkout@v2.3.4...v3)

---
updated-dependencies:
- dependency-name: actions/checkout
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
shotonoff added a commit that referenced this pull request Mar 17, 2023
* refactor: add sendMessage method to consensus.State

* refactor: modify sendMessage

* refactor: decompose WAL interface on a few small

* refactor: refactor an approach of message consuming for consensus state component

* refactor: add graceful stop for chanMsgReader

* refactor: consensus state priv-validator

* refactor: huge refactoring of consensus state, separate each state change task as a separate command, similar to state machine implementation

* refactor: add a stop function to a consensus to be able to stop receiveRoutine by a condition

* fix: remove block validating in finalizeCommit

* refactor: define Executor and VoteExtender interfaces, add executor mock, refactor return statement of Executor.ExtendVote

* test: add TestChanQueue, TestChanMsgSender, TestChanMsgReader

* test: generate PrivValidator mock

* refactor: move sending stats data from message handler

* refactor: add-vote operation (#558)

* refactor: separate TryAddVote.addVote logic on small function with one responsibility, use 2 sets (prevote and precommit) to process adding of vote
* refactor: replace peerID on proTxHash for peer catchup rounds in HeightVoteSet component
* fix: remove unused errors and StateData.validateVote

* refactor: EventSwitch implementation (#569)

* refactor: package lib/events rename on eventemitter and EventSwitch rename on EventEmitter, change EventEmitter api

* fix: add PeerManager.Unsubscribe method (#594)

* refactor: set and decide proposal operations (#565)

* refactor: set and decide proposal operations move to the separated component proposaler

* refactor: consensus peer gossip mechanism (#551)

* refactor: separate a gossip logic on implementation and a standard way of iterative handling a gossip condition

* chore: remove unless commented line

* refactor: remove custom "peerSyncErr" error

* refactor: add Gossiper interface, generate Gossiper mock struct, add unit test to queryMaj23GossipHandler

* refactor: peerGossipWorker implements service.Service

* refactor: blocksync module is refactored on using worker-pool

* test: replace custom assertError on tmrequire.Error

* refactor: add Now function into flowrate package, to obtain the current time

* fix: TestHandshakeErrorsIfAppReturnsWrongAppHash in replay_test.go

* refactor: move the logic wait-for blockchain sync from reactor to BlockPool

* fix: TestReactor_SyncTime

* test: change timing for test TestBlockPoolBasic

* chore: replace clock on clockwork

* refactor: replace reactor's consuming on separate consumer function in Channel, rename SetPeerRange on AddPeer

* test: add TestConsumeError

* chore: remove unused mustHexToBytes

* chore: add examples in promise/example_test.go and docs comments

* chore: change a package for MockValidatorSet from types on factory

* Update internal/blocksync/channel.go

* feat: create p2p/client package (#602)

* refactor: add internal/p2p/client implementation

* refactor: change blocksync module to support p2p client

* test: fix TestMakeHTTPDialerURL (#605)

* refactor: rename RandomNode on AnyNode as more specific name

* refactor: introduce p2p client in mempool module (#606)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants