Skip to content

Commit

Permalink
[core] Fixed thread safety using WSAOVERLAPPED in WSASendTo(Haivision…
Browse files Browse the repository at this point in the history
…#2838).

The lpOverlapped parameter must be valid for the duration of the overlapped operation. If multiple I/O operations are simultaneously outstanding, each must reference a separate WSAOVERLAPPED structure.

This reverts commit b1c0be2.

resolves Haivision#2632 Haivision#2834 Haivision#2838
  • Loading branch information
mGaosi committed Jan 22, 2024
1 parent 2eb47e3 commit 2065b69
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 28 deletions.
65 changes: 40 additions & 25 deletions srtcore/channel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -143,14 +143,6 @@ srt::CChannel::CChannel()
, m_bBindMasked(true)
#endif
{
#ifdef _WIN32
SecureZeroMemory((PVOID)&m_SendOverlapped, sizeof(WSAOVERLAPPED));
m_SendOverlapped.hEvent = WSACreateEvent();
if (m_SendOverlapped.hEvent == NULL) {
LOGC(kmlog.Error, log << CONID() << "IPE: WSACreateEvent failed with error: " << NET_ERROR);
throw CUDTException(MJ_SETUP, MN_NORES, NET_ERROR);
}
#endif
#ifdef SRT_ENABLE_PKTINFO
// Do the check for ancillary data buffer size, kinda assertion
static const size_t CMSG_MAX_SPACE = sizeof (CMSGNodeIPv4) + sizeof (CMSGNodeIPv6);
Expand All @@ -166,12 +158,7 @@ srt::CChannel::CChannel()
#endif
}

srt::CChannel::~CChannel()
{
#ifdef _WIN32
WSACloseEvent(m_SendOverlapped.hEvent);
#endif
}
srt::CChannel::~CChannel() {}

void srt::CChannel::createSocket(int family)
{
Expand Down Expand Up @@ -786,34 +773,62 @@ int srt::CChannel::sendto(const sockaddr_any& addr, CPacket& packet, const socka

const int res = (int)::sendmsg(m_iSocket, &mh, 0);
#else
DWORD size = (DWORD)(CPacket::HDR_SIZE + packet.getLength());
DWORD size = (DWORD)(packet.m_PacketVector[0].size() + packet.m_PacketVector[1].size());
int addrsize = addr.size();
class WSAEventRef
{
public:
WSAEventRef()
: e(::WSACreateEvent())
{
}
~WSAEventRef()
{
::WSACloseEvent(e);
e = NULL;
}
void reset()
{
::WSAResetEvent(e);
}
WSAEVENT Handle()
{
return e;
}

int res = ::WSASendTo(m_iSocket, (LPWSABUF)packet.m_PacketVector, 2, &size, 0, addr.get(), addrsize, &m_SendOverlapped, NULL);

private:
WSAEVENT e;
};
#if HAVE_CXX11
thread_local WSAEventRef lEvent;
#else
WSAEventRef lEvent;
#endif
WSAOVERLAPPED overlapped;
::SecureZeroMemory(&overlapped, sizeof(overlapped));
overlapped.hEvent = lEvent.Handle();
int res = ::WSASendTo(m_iSocket, (LPWSABUF)packet.m_PacketVector, 2, &size, 0, addr.get(), addrsize, &overlapped, nullptr);
if (res == SOCKET_ERROR)
{
if (NET_ERROR == WSA_IO_PENDING)
{
res = WSAWaitForMultipleEvents(1, &m_SendOverlapped.hEvent, TRUE, 100 /*ms*/, FALSE);
if (res == WAIT_FAILED)
DWORD dwFlags = 0;
const bool bCompleted = ::WSAGetOverlappedResult(m_iSocket, &overlapped, &size, TRUE, &dwFlags);
if (bCompleted)
{
LOGC(kslog.Warn, log << "CChannel::WSAWaitForMultipleEvents: failed with " << NET_ERROR);
res = -1;
res = 0;
}
else
{
DWORD dwFlags = 0;
const bool bCompleted = WSAGetOverlappedResult(m_iSocket, &m_SendOverlapped, &size, false, &dwFlags);
res = bCompleted ? 0 : -1;
LOGC(kslog.Error, log << "CChannel::sendto call on ::WSAGetOverlappedResult failed with error: " << NET_ERROR);
}
lEvent.reset();
}
else
{
LOGC(kmlog.Error, log << CONID() << "WSASendTo failed with error: " << NET_ERROR);
}
}
WSAResetEvent(m_SendOverlapped.hEvent);
res = (0 == res) ? size : -1;
#endif

Expand Down
3 changes: 0 additions & 3 deletions srtcore/channel.h
Original file line number Diff line number Diff line change
Expand Up @@ -176,9 +176,6 @@ class CChannel

private:
UDPSOCKET m_iSocket; // socket descriptor
#ifdef _WIN32
mutable WSAOVERLAPPED m_SendOverlapped;
#endif

// Mutable because when querying original settings
// this comprises the cache for extracted values,
Expand Down

0 comments on commit 2065b69

Please sign in to comment.