[tests] Assert that only one NetworkThread exists #11849

pull jnewbery wants to merge 4 commits into bitcoin:master from jnewbery:fix_example_test changing 23 files +96 −42

jnewbery commented at 6:45 PM on December 7, 2017: member

Add assert that only one NetworkThread exists at any time in functional tests, and fix cases where that wasn't true.

fixes #11776
laanwj added the label Tests on Dec 7, 2017
meshcollider commented at 8:03 PM on December 7, 2017: contributor

utACK https://github.com/bitcoin/bitcoin/pull/11849/commits/5a201ede58e68a1f3c5301b2f404fc143ec39bd9
ryanofsky commented at 8:09 PM on December 7, 2017: member

Seems like an easy mistake to make. Could you add a check to make it fail reliably?
jnewbery force-pushed on Dec 8, 2017
jnewbery commented at 5:03 PM on December 8, 2017: member

Seems like an easy mistake to make. Could you add a check to make it fail reliably?

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 renamed this:
~~[tests] fix example_test~~
[tests] Assert that only one NetworkThread exists
on Dec 8, 2017

[tests] Add network_thread_ utility functions.

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.

5fc6e71d19

[tests] Use network_thread_start() in tests. 74e64f24b8

[tests] Fix network threading in functional tests

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.

34e08b3510

[tests] Add NetworkThread assertions

Check that P2PConnections aren't created after the NetworkThread
has started and that at any time only one NetworkThread is running.

5c8ff26b2f

in test/functional/test_framework/mininode.py:428 in 08a6900b13 outdated
```
 423 | +def network_thread_start():
 424 | +    """Start the network thread."""
 425 | +    # Only one network thread may run at a time
 426 | +    assert not network_thread_running()
 427 | +
 428 | +    NetworkThread().start()
```
promag commented at 11:05 AM on December 10, 2017:

Why not keep this instance and check for it, instead of enumerating threads with a custom name?

jnewbery commented at 1:08 PM on December 10, 2017:

I can't see how that's any better. Perhaps you can push a branch to demonstrate what you mean.
jnewbery force-pushed on Dec 11, 2017
jnewbery commented at 2:24 PM on December 11, 2017: member

rebased
laanwj commented at 12:09 PM on December 12, 2017: member

Good idea. utACK 5c8ff26b2fc59d8bcfa2a8bfb7c9fe6d78cf6678
laanwj merged this on Dec 12, 2017
laanwj closed this on Dec 12, 2017
laanwj referenced this in commit ad1820cbad on Dec 12, 2017
codablock referenced this in commit 5afd4ad7b9 on Sep 30, 2019
codablock referenced this in commit 55b6d1a168 on Sep 30, 2019
codablock referenced this in commit 89ca365b3f on Oct 2, 2019
codablock referenced this in commit d9e6320713 on Oct 2, 2019
UdjinM6 referenced this in commit 2b47a25ce9 on Oct 3, 2019
codablock referenced this in commit f4ea836744 on Oct 3, 2019
barrystyle referenced this in commit a9f35bdf23 on Jan 22, 2020
DrahtBot locked this on Sep 8, 2021

Contributors

Labels

Linked (view graph)

#11776 [qa] example_test.py failed