[tests] Assert that only one NetworkThread exists #11849
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
|
utACK 5a201ed |
Contributor
|
Seems like an easy mistake to make. Could you add a check to make it fail reliably? |
jnewbery
force-pushed
the
fix_example_test
branch
from
5a201ed to
08a6900
Compare
Contributor
Author
Excellent idea. I've repurposed this PR to add checking that only one NetworkThread is created. The extra checks have caught three additional potential bugs/races. |
jnewbery
changed the title
promag
reviewed
Dec 10, 2017
Contributor
There was a problem hiding this comment.
Why not keep this instance and check for it, instead of enumerating threads with a custom name?
Contributor
Author
There was a problem hiding this comment.
I can't see how that's any better. Perhaps you can push a branch to demonstrate what you mean.
Add network thread_start(), network_thread_running() and network_thread_join() utility functions in mininode.py and use network_thread_running() in network thread assertions.
assumevalid.py, example_test.py and p2p-acceptblocks.py add p2p_connections after the NetworkThread has been started. This isn't permitted. Fix test to restart the network thread when adding new connections. p2p-leaktest.py had a potential race condition if the NetworkThread hadn't terminated by the time we tried to restart it.
Check that P2PConnections aren't created after the NetworkThread has started and that at any time only one NetworkThread is running.
jnewbery
force-pushed
the
fix_example_test
branch
from
08a6900 to
5c8ff26
Compare
Contributor
Author
|
rebased |
Member
|
Good idea. utACK 5c8ff26 |
laanwj
added a commit
that referenced
this pull request
Dec 12, 2017
5c8ff26 [tests] Add NetworkThread assertions (John Newbery) 34e08b3 [tests] Fix network threading in functional tests (John Newbery) 74e64f2 [tests] Use network_thread_start() in tests. (John Newbery) 5fc6e71 [tests] Add network_thread_ utility functions. (John Newbery) Pull request description: Add assert that only one NetworkThread exists at any time in functional tests, and fix cases where that wasn't true. fixes #11776 Tree-SHA512: fe5d1c59005f94bf66e11bb23ccf274b1cd9913741b56ea11dbcd21db4cc0b53b4413c0c4c16dbcd6ac611adad5e5cc2baaa39720598ce7b6393889945d06298
codablock
pushed a commit
to codablock/dash
that referenced
this pull request
Oct 3, 2019
5c8ff26 [tests] Add NetworkThread assertions (John Newbery) 34e08b3 [tests] Fix network threading in functional tests (John Newbery) 74e64f2 [tests] Use network_thread_start() in tests. (John Newbery) 5fc6e71 [tests] Add network_thread_ utility functions. (John Newbery) Pull request description: Add assert that only one NetworkThread exists at any time in functional tests, and fix cases where that wasn't true. fixes bitcoin#11776 Tree-SHA512: fe5d1c59005f94bf66e11bb23ccf274b1cd9913741b56ea11dbcd21db4cc0b53b4413c0c4c16dbcd6ac611adad5e5cc2baaa39720598ce7b6393889945d06298
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add assert that only one NetworkThread exists at any time in functional tests, and fix cases where that wasn't true.
fixes #11776