test fix: keep connection alive

[bw-solana]

Problem

test_quic_server_multiple_connections_on_single_client_endpoint is flaky in CI. For example, see https://buildkite.com/anza/agave/builds/31839#0199de40-d9cd-4a5d-b814-1a48e19f8d23

We rely on this receiver being alive to keep the server alive, but we don't use the receiver so we drop it immediately.

There seems to be some timing/coalesce variance that will occasionally cause the following sequence to happen:

1. server tries to send
2. recognizes channel is disconnected because receiver dropped
3. calls cancel on the cancellation token
4. connection gets dropped

This can then lead to multiple different failure modes in this test (and maybe others) depending on exact timing:

1. We might jump up to 2 removed connections and spin forever in this loop because we're explicitly waiting for 1 removed connection
2. We might try and write to the connection, fail, try to unwrap the error, and panic

I'm guessing #8025 is what caused this issue to start popping up

Summary of Changes

Explicitly drop the receiver at the end of the test to prevent dropping it earlier

[bw-solana]

@KirillLykov It seems like there are other tests that might need a similar fix

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.2%. Comparing base (d3247fa) to head (e738a15).

Additional details and impacted files

@@            Coverage Diff            @@
##           master    #8458     +/-   ##
=========================================
- Coverage    83.2%    83.2%   -0.1%     
=========================================
  Files         841      841             
  Lines      368425   368430      +5     
=========================================
- Hits       306635   306615     -20     
- Misses      61790    61815     +25     

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[KirillLykov]

interesting, before it was exit.store(true, Ordering::Relaxed); instead of cancel.cancel(); when receiver dropped in run_packet_batch_sender. But before we didn't kill the tasks handling connections when cancellation happens, maybe that's why.

[KirillLykov]

Will check out if we have other tests like that.

[steviez]

Good catch! I feel like I might have known this could happen at some point but didn't remember this time around. Had to dig a little to find this in the book:

Note that there is a subtle difference between using only _ and using a name that starts with an underscore. The syntax _x still binds the value to the variable, whereas _ doesn’t bind at all. To show a case where this distinction matters, Listing 19-21 will provide us with an error.
...
However, using the underscore by itself doesn’t ever bind to the value

Even then, I'm surprised the docs aren't a little more direct about this potential footgun. Some issues in rust-lang would suggest that other people find this bit a little tricky too but 🤷

FWIW, it looks like doing receiver: _receiver would have sufficed too but the explicit drop() at the end does the job as well !

[bw-solana]

yeah, such a foot gun... I'm happy to change this to _receiver as opposed to the explicit drop at the end.

[alexpyattaev]

Explicit is better than implicit.

[KirillLykov]

I think we also should not wait without timeout for some atomics to have desired value. Will fix this as well.