Fix accidental spinloop in connection accept logic

[rukai]

Problem

During implementation of the hot reload functionality, a spin loop was accidentally introduced into the connection acception loop.
This does not impact shotovers ability to function, as evidenced by all the tests continuing to pass, but it is a huge performance concern as 1 CPU core will always be spent in a loop.

I discovered the problem because the connection IDs were jumping to ridiculously high values in the integration tests. Here you can see a connection ID of 12787950, which implies that 12787950 connections have been made during this test.

When in reality a driver will only make ~10 connections during a lengthy test.

Cause

The loop is this one here:

shotover-proxy/shotover/src/server.rs

Line 204 in 7a7f863

loop {

The tokio select found here, will wait until either:

• a connection has come in that we need to accept
• a hot reload request has come in that needs to be processed.

However when hot reload is disabled (the default configuration) the hot_reload_rx channel is closed, and so the hot reload branch of the select will always immediately trigger. When the hot reload branch triggers, nothing happens, the loop ends, and starts from the beginning again.

Why is hot_reload_rx closed?

The other end of the hot_reload_rx is the hot_reload_tx, which is stored in Source:

shotover-proxy/shotover/src/sources/mod.rs

Lines 37 to 42 in 7a7f863

	pub struct Source {
	pub join_handle: JoinHandle<()>,
	pub hot_reload_tx: UnboundedSender<HotReloadListenerRequest>,
	pub gradual_shutdown_tx: UnboundedSender<GradualShutdownRequest>,
	pub name: String,
	}

Source::into_join_handle consumes the Source, dropping everything inside the Source, except for the join_handle which is returned. You can tell it consumes Source because the method argument is self instead of &self. see: https://google.github.io/comprehensive-rust/methods-and-traits/methods.html

shotover-proxy/shotover/src/sources/mod.rs

Lines 59 to 61 in 7a7f863

	pub fn into_join_handle(self) -> JoinHandle<()> {
	self.join_handle
	}

As a result, hot_reload_tx is dropped during shotover setup, closing the channel.

The fix

The fix is subtle, but the goal here is to ensure that the hot_reload_tx remains alive for as long as the listener background task is alive.
We do this by changing the into_join_handle into an async method, this ensures that the rest of Source is kept alive for the lifetime of the listener task.

Technically the std::mem::drop(self.hot_reload_tx); isnt required, since the drop would occur implicitly at the end of the function, but since this issue is subtle, I figure its better to be explicit here.

[codspeed-hq]

Merging this PR will not alter performance

✅ 38 untouched benchmarks

---

_{Comparing rukai:fix_accidental_spin_loop (9b79cae) with main (7a7f863)}

[yallen-ic]

LGTM