neqo icon indicating copy to clipboard operation
neqo copied to clipboard

Mark WebTransport streams keep-alive

Open ddragana opened this issue 2 years ago • 2 comments

We need to propagate neqo_transport::Connection to this function and call this function: https://github.com/mozilla/neqo/blob/25a862e97072a06b2ac93a6d5035e0202c0c7d6f/neqo-http3/src/connection.rs#L1048

ddragana avatar Oct 06 '22 09:10 ddragana

I tried propagating neqo_transport::Connection and calling conn.stream_keep_alive(stream_id, true)?. This caused panic for the following tests in streams.rs.

wt_client_stream_uni
wt_server_stream_uni

The panic is originating from RecvStreams::keep_alive:

 pub fn keep_alive(&mut self, id: StreamId, k: bool) -> Res<()> {             
  65         let self_ka = &mut self.keep_alive;                                      
  66         let s = self.streams.get_mut(&id).ok_or(Error::InvalidStreamId)?;                                                                                                                                                                                            
  

FWIU, we shouldnt enable the keep alive flags for local unidirectional streams as they do not have recv streams associated with the session. Hence, we should enable keep alive flag for the following streams:

  • Bidirectional - local and remote
  • Unidirectional - remote

@KershawChang, @martinthomson : Please confirm if my understanding is correct

MayyaSunil avatar Dec 01 '22 11:12 MayyaSunil

@MayyaSunil you should only need to enable a keep-alive for the session stream, that is the "request" that is still open. If that stream closes, the whole WebTransport session ends, so - that way - you will be keeping the connection alive as long as there is any WebTransport session active.

martinthomson avatar Dec 01 '22 14:12 martinthomson