-
Notifications
You must be signed in to change notification settings - Fork 865
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[core] WSASend overlapped #974
[core] WSASend overlapped #974
Conversation
// Wait for 20 ms | ||
int rc = WSAWaitForMultipleEvents(1, &m_SendOverlapped.hEvent, TRUE, 50, TRUE); | ||
|
||
if (rc == WSA_WAIT_FAILED) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Convention.
m_SendOverlapped.hEvent = WSACreateEvent(); | ||
if (m_SendOverlapped.hEvent == NULL) { | ||
LOGC(mglog.Error, log << CONID() << "IPE: WSACreateEvent failed with error: " << NET_ERROR); | ||
throw CUDTException(MJ_SETUP, MN_NONE, NET_ERROR); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the minor code should be MN_NORES
, as the new CChannel
occurs only once in updateMux
, which is called directly from bind
and connect
calls. Note that the documentation would have to be updated.
6147294
to
7492746
Compare
Rebased on the latest master ☝️ |
|
Added check of the
WSASend
result, as reported in #973.Given that
::send(...)
wan not observed to fail on Linux, adding a common::select(...)
call for both Windows and Linux might decrease the performance.This PR uses Windows OverlappedIO API to wait for
WSASend
to suceed.50 ms waiting time was chosen empiricaly. 10 ms produce more failures on sending.