private-octopus / picoquic

Minimal implementation of the QUIC protocol
MIT License
547 stars 161 forks source link

Stream context is freed before FIN byte is acknowledged #1515

Closed huitema closed 1 year ago

huitema commented 1 year ago

The stream context includes a summary of the stream segments that have already been received, as exposed through the API

picoquic_check_sack_list(&stream->sack_list, offset, offset + data_available)

This API allows the senders to avoid repeating stream segments that have already been acknowledged. However, the API assumes that the stream context is still available, which is not always true: the stream context is currently deleted after the FIN flags has been sent in the "send" direction and received in the other direction -- with suitable arrangements for unidir streams.

If stream data frames are still in flight at that point, the code cannot test the summary of data received in the stream context, and has no choice but to repeat the data until every copy is acknowledged. This is quite inefficient.

Proposed fix:

huitema commented 1 year ago

Then, update picoquic_copy_stream_frame_for_retransmit to not retransmit frames if the stream context is deleted, and to check that the "final offset+1" byte is acknowledged if the FIN flag is set.