How to reject incoming connections after quinn 0.9?

[ecton]

I'm trying to upgrade a library that uses quinn under the hood, and the library previously offered the ability to close incoming connections to enable a way to gracefully shut down a server. This worked in 0.8 by passing the Incoming stream into its own task, and stopping that task once close_incoming was called. This dropped the Incoming type, which caused incoming connections to be refused.

In quinn 0.9 and later, Endpoint::server no longer returns an Incoming type, and the Endpoint now has an accept() function instead. All of my attempts to prevent incoming connections have not worked: either the incoming connection is accepted before being terminated or the connection hangs rather than being refused.

Is there a way to preserve the functionality we were utilizing in 0.8 in the current version? Thank you, and apologies if I missed something in the docs for how to handle this!

[djc]

So we had impl Drop for Incoming in 0.8.5, but since Incoming was removed in #1426, we also lost this capability (which seems to have been added in #273). I think it would make sense to add an fn reject_new_connections(self) to quinn::Endpoint, would that satisfy your use case? Would you be able to submit a PR, ideally including a test to help deter us from removing it again?

[ecton]

I would be happy to!

[Ralith]

This functionality actually already exists: via Endpoint::set_server_config you can adjust the concurrent_connections limit to 0 (producing CONNECTION_REFUSED errors on future attempts).

[ecton]

This functionality actually already exists: via Endpoint::set_server_config you can adjust the concurrent_connections limit to 0 (producing CONNECTION_REFUSED errors on future attempts).

Once I saw how this was implemented, I thought about that, but it didn't feel very ergonomic. Since Endpoint doesn't give an accessor to read the server config, it would require consumers to keep a copy of their server config around.

[Ralith]

I'd prefer to address an ergonomic shortfall there than to set a precedent of duplicating ServerConfig's setters on Endpoint. Would it suit you to for there to be a getter for an Arc<ServerConfig>?

[ecton]

That's really only half of the problem. I'm happy to accept that as a solution. I'm also happy to accept no changes at all, now that I understand how to accomplish this use case. The problem for other users is discoverability of this functionality. With the current pull request, an approach to do a graceful shutdown of a server connection is clearly available.

Without a dedication function, the flow of how to shutdown incoming connections isn't very clear, even if access to the server config is provided. I'd at a minimum suggest documenting this flow as part of wait_idle's final paragraph.

From a usability standpoint, I personally would prefer a dedicated function and the accessor, rather than just one or the other. Since I can technically do this today by storing my server config, I'll be updating the library without needing a new release. That means I really don't mind whichever approach is chosen -- I'm unblocked :)

[Ralith]

It's a fair point that this is a uniquely important/common pattern compared to other config changes, and I'm open to proceeding with the change on that basis. Will follow up in the PR.