Add logic to handle partial hvsocket writes and additional logging#13602
Add logic to handle partial hvsocket writes and additional logging#13602
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds robust handling for partial writes to hvsockets, which may be a root cause for issue #13301. The changes ensure that when a socket write operation doesn't complete in a single call, the system continues writing the remaining data until the full buffer is transmitted.
- Implements a loop in the Send function to handle partial writes by tracking offset and continuing until all data is sent
- Adds comprehensive logging for partial writes and socket connection failures
- Reorganizes header includes to ensure logging macros are available when needed
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/windows/common/socket.cpp | Refactors Send function to handle partial writes with retry loop and enhanced error handling |
| src/shared/inc/socketshared.h | Adds detailed logging when socket closes during message reading |
| src/shared/inc/SocketChannel.h | Updates message sending to capture and log bytes sent, fixes syntax error |
| src/linux/init/common.h | Reorders includes to ensure logging macros are defined before shared headers that use them |
Comments suppressed due to low confidence (1)
src/shared/inc/socketshared.h:1
- Corrected spelling of 'MessagSize' to 'MessageSize'.
/*++
| @@ -80,6 +80,27 @@ try | |||
| #endif | |||
| if (BytesRead <= 0) | |||
| { | |||
Copilot
AI
Oct 15, 2025
•
There was a problem hiding this comment.
Unsafe cast without validating buffer size. The buffer should be checked to ensure it contains at least sizeof(MESSAGE_HEADER) bytes before casting to avoid potential buffer overrun when accessing Header fields.
| { | |
| { | |
| if (Buffer.size() < sizeof(MESSAGE_HEADER)) { | |
| LOG_ERROR("Buffer too small for MESSAGE_HEADER: size = {}", Buffer.size()); | |
| return {}; | |
| } | |
| ``` #Resolved |
There was a problem hiding this comment.
@OneBlue - I think this one is valid. Could you also use try_get_struct?
There was a problem hiding this comment.
I don't think it is. We resize the buffer to the header size in the beginning:
auto MessageSize = sizeof(MESSAGE_HEADER);
if (Buffer.size() < MessageSize)
{
Buffer.resize(MessageSize);
}
We also don't have a gsl range that we can use in this block
There was a problem hiding this comment.
Ah fair I see this buffer is at least the size of header.
There was a problem hiding this comment.
But isn't "Message" the range?
There was a problem hiding this comment.
Message points to the end of the buffer:
Message = gsl::make_span(Buffer.data(), MessageSize).subspan(sizeof(MESSAGE_HEADER));
So it wouldn't allow us to access the message header
Summary of the Pull Request
This change adds logic to correctly handle a partial write to an hvsocket and logs when that happens.
This might be the root cause to #13301
PR Checklist
Detailed Description of the Pull Request / Additional comments
Validation Steps Performed