Skip to content

Commit

Permalink
Should fix #333:
Browse files Browse the repository at this point in the history
- Do not close RTC::UdpSocket or RTC::TcpConnection when
  uv_udp_try_send() or uv_try_write() fail due to an unexpected error
  other than EAGAIN.
- Otherwise, while reading from a TcpConnection it my happen that there
  are more than a single "packet". If the first packet is a STUN request
  and the IceServer generates a sync response for it, and
  TcpConnection::Write() fails for unknown reason and we delete the
  TcpConnection, the reading process would continue with the second
  "packet", meaning that we are in a deallocated TcpConnection
  instance and BUMP!
  • Loading branch information
ibc committed Nov 25, 2019
1 parent 48837cb commit 49824ba
Show file tree
Hide file tree
Showing 4 changed files with 15 additions and 47 deletions.
7 changes: 5 additions & 2 deletions worker/src/RTC/TcpConnection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ namespace RTC
// We may receive multiple packets in the same TCP chunk. If one of them is
// a DTLS Close Alert this would be closed (Close() called) so we cannot call
// our listeners anymore.
if (::TcpConnection::IsClosed())
if (IsClosed())
return;

size_t dataLen = this->bufferDataLen - this->frameStart;
Expand Down Expand Up @@ -138,7 +138,10 @@ namespace RTC
"connection");

// Close the socket.
::TcpConnection::ErrorReceiving();
ErrorReceiving();

// And exit fast since we are supposed to be deallocated.
return;
}
}
// The buffer is not full.
Expand Down
34 changes: 6 additions & 28 deletions worker/src/handles/TcpConnection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -232,21 +232,10 @@ void TcpConnection::Write(const uint8_t* data, size_t len, TcpConnection::onSend
// Error. Should not happen.
else if (written < 0)
{
MS_WARN_DEV("uv_try_write() failed, closing the connection: %s", uv_strerror(written));
MS_WARN_DEV("uv_try_write() failed, trying uv_write(): %s", uv_strerror(written));

if (cb)
{
(*cb)(false);

delete cb;
}

Close();

// Notify the listener.
this->listener->OnTcpConnectionClosed(this);

return;
// Set written to 0 so pendingLen can be properly calculated.
written = 0;
}

// MS_DEBUG_DEV(
Expand Down Expand Up @@ -351,21 +340,10 @@ void TcpConnection::Write(
// Error. Should not happen.
else if (written < 0)
{
MS_WARN_DEV("uv_try_write() failed, closing the connection: %s", uv_strerror(written));
MS_WARN_DEV("uv_try_write() failed, trying uv_write(): %s", uv_strerror(written));

if (cb)
{
(*cb)(false);

delete cb;
}

Close();

// Notify the listener.
this->listener->OnTcpConnectionClosed(this);

return;
// Set written to 0 so pendingLen can be properly calculated.
written = 0;
}

size_t pendingLen = totalLen - written;
Expand Down
11 changes: 1 addition & 10 deletions worker/src/handles/UdpSocket.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -179,16 +179,7 @@ void UdpSocket::Send(
// Error,
if (sent != UV_EAGAIN)
{
MS_WARN_DEV("uv_udp_try_send() failed: %s", uv_strerror(sent));

if (cb)
{
(*cb)(false);

delete cb;
}

return;
MS_WARN_DEV("uv_udp_send() failed, trying uv_udp_send(): %s", uv_strerror(sent));
}

// MS_DEBUG_DEV("could not send the datagram at first time, using uv_udp_send() now");
Expand Down
10 changes: 3 additions & 7 deletions worker/src/handles/UnixStreamSocket.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -189,14 +189,10 @@ void UnixStreamSocket::Write(const uint8_t* data, size_t len)
// Error. Should not happen.
else if (written < 0)
{
MS_ERROR_STD("uv_try_write() failed, closing the socket: %s", uv_strerror(written));
MS_WARN_DEV("uv_try_write() failed, trying uv_write(): %s", uv_strerror(written));

Close();

// Notify the subclass.
UserOnUnixStreamSocketClosed();

return;
// Set written to 0 so pendingLen can be properly calculated.
written = 0;
}

size_t pendingLen = len - written;
Expand Down

0 comments on commit 49824ba

Please sign in to comment.