Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add function to check if connection/stream is open #2514

Open
CorneliusCornbread opened this issue Mar 5, 2025 · 5 comments
Open

Add function to check if connection/stream is open #2514

CorneliusCornbread opened this issue Mar 5, 2025 · 5 comments
Labels
enhancement New feature or request

Comments

@CorneliusCornbread
Copy link

CorneliusCornbread commented Mar 5, 2025

Problem:

When creating a stream if you close it's related Connection there is no way to check via the Stream if the Stream is still open/valid. This can be a problem if you are assigning a Stream to their async task but handling Connection management behaviour in another task, especially if you split a Stream into a SendStream and a ReceiveStream and handle their respective logic on separate tasks. If you close a connection in one task, there needs to be a way for all related streams to know that their stream has been closed.

Solution:

A simple function within a Connection and respective Stream structs to query the connection's status would solve this problem quite easily.

if !stream.is_open() {
  // Handle dead stream gracefully without having to call something like `send()` with garbage data or `recv()` and possibly blocking when receiving
}
  • Does this change what s2n-quic sends over the wire?

  • Nope, this functionality should already exist within send() and receive() in theory as these functions can error if the stream has been disconnected

  • Does this change any public APIs?

  • Yes, connections and streams will have a new function, this shouldn't conflict with existing usages of s2n-quic though as it's an addition.

Requirements / Acceptance Criteria:

  • Will the Usage Guide or other documentation need to be updated?
  • Yes, there will need to be a some documentation for this new function, a code example though is likely unnecessary given the simplicity of the function.
  • Testing: How will this change be tested? Call out new integration tests, functional tests, or particularly interesting/important unit tests.
  • Create a test that creates a valid s2n-quic connection between a client and server, open a stream, then disconnect the parent connection. Call the stream's is_open() function and verify it returns false (or whatever return type/value that signifies a closed connection)

Original Question

So I've ran into an issue with the way I've architected my usage of s2n-quic. The way I have it setup is I have two long running tasks, one of which is a send task that has a tokio channel to send data to a given target via a split bidirectional stream, the other is a receive task that accepts new connections, opens a stream and receives data from the stream.

The problem with this is that if my receiver task closes a connection the send stream, unless I explicitly add an extra messaging channel for this, won't know the connection has been closed. This results in essentially leaking memory in certain scenarios as the send task will continue to hold onto this SendStream unless it attempts to send data to the target and get a send error, I'd really prefer to not have to manually ping all my streams on a regular basis to check if a stream is dead. And from the docs there doesn't seem to be a way to check if a Send/ReceiveStream is still open.

I could solve this by changing it such that the sending and receiving happens all in one async function call but then I'd have to somehow block and synchronize the the open connections after closing or opening one, which could impact response times, especially when handling many streams/connections.

@boquan-fang boquan-fang added the question Further information is requested label Mar 6, 2025
@boquan-fang
Copy link
Contributor

This functionality doesn't exist in S2N-QUIC today. The connection would need to provide a way to subscribe to closure event.

@CorneliusCornbread
Copy link
Author

This functionality doesn't exist in S2N-QUIC today. The connection would need to provide a way to subscribe to closure event.

It would be preferable to have a function that could be used to check the status. Having to send that information from a subscriber function call to the owner of the stream would be a lot of boilerplate and a huge hassle.

@camshaft
Copy link
Contributor

camshaft commented Mar 6, 2025

The subscribe functionality @boquan-fang was mentioning was more along the lines of what you're saying where you would have a function that "subscribes" via a future that resolves once the connection is closed, for any reason.

race(connection.wait_close(), channel.recv()).await;

@CorneliusCornbread
Copy link
Author

The subscribe functionality @boquan-fang was mentioning was more along the lines of what you're saying where you would have a function that "subscribes" via a future that resolves once the connection is closed, for any reason.

race(connection.wait_close(), channel.recv()).await;

I guess I should ask this then, should I reformat this question issue to be instead a feature request?

@camshaft
Copy link
Contributor

should I reformat this question issue to be instead a feature request?

Yes, that would be great. Thanks!

@camshaft camshaft added enhancement New feature or request and removed question Further information is requested labels Mar 11, 2025
@CorneliusCornbread CorneliusCornbread changed the title How to check if stream/connection has been closed Add function to check if connection/stream is open Mar 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants