Intermittent issue in p2p_addr_relay (AssertionError: not(16 == 20)) #22243

https://cirrus-ci.com/task/4899698315100160?logs=ci#L5292

                                     File "/tmp/cirrus-ci-build/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/functional/p2p_addr_relay.py", line 123, in relay_tests
                                       assert_equal(total_ipv4_received, num_ipv4_addrs * ipv4_branching_factor)
                                     File "/tmp/cirrus-ci-build/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/functional/test_framework/util.py", line 51, in assert_equal
                                       raise AssertionError("not(%s)" % " == ".join(str(arg) for arg in (thing1, thing2) + args))
                                   AssertionError: not(16 == 20)

I suspect that this issue is caused because the test thread is accessing num_ipv4_received on the P2PInterface object without first acquiring the p2p_lock (link). So there might be a rare-but-possible race between the asyncio threads receiving messages / updating the value & the test logic reading from the value.

~I'll open a PR with the fix. I currently have a commit in #21528 to refactor this test, I'm going to pull it out into a standalone PR with the additional locking.~ (No longer going to do this based on Marco's comment below)

I think two thread accessing the same python symbol is impossible in the "normal" python due to the GIL (python global interpreter lock), so I think the p2p_lock exists only to look nice, but is practically useless. If there is a racy test it should be fixed by adding a synchronisation primitive (like a notification, flush, polling, ...) potentially in combination with mocktime.

I'm pretty convinced the issue is because of the poisson distribution of the m_next_addr_send timer. I opened #22306 with the mocktime bump + some other refactoring to the test file.