TLS deliver buffer data during shutdown

[TingDaoK]

Issue #, if available:

• Very similar to Fix bug where last few bytes on socket go unread #642
• In this case:
  • the downstream window is limiting the data to be delivered from TLS, so TLS handler will cache the socket data.
  • While the window update task is pending for channel to pick up, socket reads EOF and start to schedule the shutdown task. So the shutdown task will run immediately after the window update happens.
  • The window update task opens the window again and schedule another read task like:

    aws-c-io/source/s2n/s2n_tls_channel_handler.c

    Lines 1072 to 1082 in 9072f86

    	if (s2n_handler->state == NEGOTIATION_SUCCEEDED && !s2n_handler->sequential_tasks.node.next) {
    	/* TLS requires full records before it can decrypt anything. As a result we need to check everything we've
    	* buffered instead of just waiting on a read from the socket, or we'll hit a deadlock.
    	*
    	* We have messages in a queue and they need to be run after the socket has popped (even if it didn't have data
    	* to read). Alternatively, s2n reads entire records at a time, so we'll need to grab whatever we can and we
    	* have no idea what's going on inside there. So we need to attempt another read.*/
    	aws_channel_task_init(
    	&s2n_handler->sequential_tasks, s_run_read, handler, "s2n_channel_handler_read_on_window_increment");
    	aws_channel_schedule_task_now(slot->channel, &s2n_handler->sequential_tasks);
    	}

    , which will happen after the shutdown task
  • So, the cached data will be delivered to downstream after the shutdown happens and result in lost of data.

Description of changes:

• TLS handler now waits for buffered data to be delivered before it completes shutdown
• Allow the window update happens during shut down
• fix a bug in windows that we may endup overflow buffer when we try to read the whole frame for TLS to decrypt, while the downstream has no window left.

Notes:

• With this fix, if downstream of TLS has 0 window and channel shuts down, it may result in hanging.
  • If the shutdown in not initialized by the downstream, it's okay, since it's expected behavior for downstream when the window reaches to 0.
  • If the shutdown is initialized by downstream, and it wants to shutdown without caring the window size. It should increase the window after shutdown starts. With CRT libs, we only used TLS handler with the channel flow-control window from HTTP/1. adapt change from "TLS deliver buffer data during shutdown" aws-c-http#474

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 96.29630% with 2 lines in your changes missing coverage. Please review.

Project coverage is 80.39%. Comparing base (bf06976) to head (463c563).

Files	Patch %	Lines
source/s2n/s2n_tls_channel_handler.c	96.15%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #650      +/-   ##
==========================================
+ Coverage   80.18%   80.39%   +0.20%     
==========================================
  Files          28       28              
  Lines        5966     5992      +26     
==========================================
+ Hits         4784     4817      +33     
+ Misses       1182     1175       -7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[graebm]

@TingDaoK I pushed my own take on the Windows error-handling. Can you take a look at it?

[TingDaoK]

looks good to me, thanks!