Closed graebm closed 1 month ago
Attention: Patch coverage is 88.88889%
with 2 lines
in your changes missing coverage. Please review.
Project coverage is 80.05%. Comparing base (
e762fd2
) to head (56d92d3
).
Files | Patch % | Lines |
---|---|---|
source/socket_channel_handler.c | 81.81% | 2 Missing :warning: |
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Issue:
aws-c-s3 is occasionally seeing errors when the server sends an HTTP response with a
Connection: close
header (meaning it intends to close the connection after the response is sent). The server is sending the full response, then immediately hanging up. But the last few bytes of the response never make it to the HTTP client.Diagnosis:
If a peer closed their socket immediately after the last few bytes were sent, our socket code wasn't always reading those last few bytes.
On Linux, if a socket has unread data AND the peer has closed their side, the event from epoll has 2 flags set:
EPOLLIN|EPOLLRDHUP
. This means "the socket is has data to read AND the other side is closed (or half-closed) and won't be sending any more data".Our socket handler code kinda did the right thing by "attempting" to read from the socket before shutting down the channel. But if the downstream read window reached 0, that "attempt" wouldn't read all the data.
Description of changes:
The socket handler no longer shuts down the channel in response to an error event. Instead, the error event queues more reads to happen. And only when
read()
reports that the socket is finished (due to error or EOF), will the socket handler shut down the channel.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.