Skip to content

Commit

Permalink
Avoid ASAN "new-delete-type-mismatch" harmless but noisy warning (#1129)
Browse files Browse the repository at this point in the history
  • Loading branch information
ibc authored Jul 28, 2023
1 parent dc4f8f3 commit 222ebd2
Show file tree
Hide file tree
Showing 10 changed files with 105 additions and 45 deletions.
7 changes: 4 additions & 3 deletions worker/src/Channel/ChannelSocket.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@ namespace Channel
}
}

inline static void onClose(uv_handle_t* handle)
inline static void onCloseAsync(uv_handle_t* handle)
{
delete handle;
delete reinterpret_cast<uv_async_t*>(handle);
}

/* Instance methods. */
Expand Down Expand Up @@ -100,7 +100,8 @@ namespace Channel

if (this->uvReadHandle)
{
uv_close(reinterpret_cast<uv_handle_t*>(this->uvReadHandle), static_cast<uv_close_cb>(onClose));
uv_close(
reinterpret_cast<uv_handle_t*>(this->uvReadHandle), static_cast<uv_close_cb>(onCloseAsync));
}

if (this->consumerSocket)
Expand Down
6 changes: 3 additions & 3 deletions worker/src/DepLibUV.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@ thread_local uv_loop_t* DepLibUV::loop{ nullptr };

/* Static methods for UV callbacks. */

inline static void onClose(uv_handle_t* handle)
inline static void onCloseLoop(uv_handle_t* handle)
{
delete handle;
delete reinterpret_cast<uv_loop_t*>(handle);
}

inline static void onWalk(uv_handle_t* handle, void* /*arg*/)
Expand All @@ -27,7 +27,7 @@ inline static void onWalk(uv_handle_t* handle, void* /*arg*/)
uv_has_ref(handle));

if (!uv_is_closing(handle))
uv_close(handle, onClose);
uv_close(handle, onCloseLoop);
}

/* Static methods. */
Expand Down
7 changes: 4 additions & 3 deletions worker/src/PayloadChannel/PayloadChannelSocket.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,9 @@ namespace PayloadChannel
}
}

inline static void onClose(uv_handle_t* handle)
inline static void onCloseAsync(uv_handle_t* handle)
{
delete handle;
delete reinterpret_cast<uv_async_t*>(handle);
}

/* Instance methods. */
Expand Down Expand Up @@ -102,7 +102,8 @@ namespace PayloadChannel

if (this->uvReadHandle)
{
uv_close(reinterpret_cast<uv_handle_t*>(this->uvReadHandle), static_cast<uv_close_cb>(onClose));
uv_close(
reinterpret_cast<uv_handle_t*>(this->uvReadHandle), static_cast<uv_close_cb>(onCloseAsync));
}

if (this->consumerSocket)
Expand Down
64 changes: 54 additions & 10 deletions worker/src/RTC/PortManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,16 @@

/* Static methods for UV callbacks. */

static inline void onClose(uv_handle_t* handle)
// NOTE: We have different onCloseXxx() callbacks to avoid an ASAN warning by
// ensuring that we call `delete xxx` with same type as `new xxx` before.
static inline void onCloseUdp(uv_handle_t* handle)
{
delete handle;
delete reinterpret_cast<uv_udp_t*>(handle);
}

static inline void onCloseTcp(uv_handle_t* handle)
{
delete reinterpret_cast<uv_tcp_t*>(handle);
}

inline static void onFakeConnection(uv_stream_t* /*handle*/, int /*status*/)
Expand Down Expand Up @@ -161,30 +168,44 @@ namespace RTC
switch (transport)
{
case Transport::UDP:
{
uvHandle = reinterpret_cast<uv_handle_t*>(new uv_udp_t());
err = uv_udp_init_ex(
DepLibUV::GetLoop(), reinterpret_cast<uv_udp_t*>(uvHandle), UV_UDP_RECVMMSG);

break;
}

case Transport::TCP:
{
uvHandle = reinterpret_cast<uv_handle_t*>(new uv_tcp_t());
err = uv_tcp_init(DepLibUV::GetLoop(), reinterpret_cast<uv_tcp_t*>(uvHandle));

break;
}
}

if (err != 0)
{
delete uvHandle;

switch (transport)
{
case Transport::UDP:
{
delete reinterpret_cast<uv_udp_t*>(uvHandle);

MS_THROW_ERROR("uv_udp_init_ex() failed: %s", uv_strerror(err));

break;
}

case Transport::TCP:
{
delete reinterpret_cast<uv_tcp_t*>(uvHandle);

MS_THROW_ERROR("uv_tcp_init() failed: %s", uv_strerror(err));

break;
}
}
}

Expand Down Expand Up @@ -259,7 +280,22 @@ namespace RTC
break;

// If it failed, close the handle and check the reason.
uv_close(reinterpret_cast<uv_handle_t*>(uvHandle), static_cast<uv_close_cb>(onClose));
switch (transport)
{
case Transport::UDP:
{
uv_close(reinterpret_cast<uv_handle_t*>(uvHandle), static_cast<uv_close_cb>(onCloseUdp));

break;
};

case Transport::TCP:
{
uv_close(reinterpret_cast<uv_handle_t*>(uvHandle), static_cast<uv_close_cb>(onCloseTcp));

break;
}
}

switch (err)
{
Expand Down Expand Up @@ -400,17 +436,25 @@ namespace RTC

if (err != 0)
{
delete uvHandle;

switch (transport)
{
case Transport::UDP:
{
delete reinterpret_cast<uv_udp_t*>(uvHandle);

MS_THROW_ERROR("uv_udp_init_ex() failed: %s", uv_strerror(err));

break;
}

case Transport::TCP:
{
delete reinterpret_cast<uv_tcp_t*>(uvHandle);

MS_THROW_ERROR("uv_tcp_init() failed: %s", uv_strerror(err));

break;
}
}
}

Expand All @@ -426,7 +470,7 @@ namespace RTC
if (err != 0)
{
// If it failed, close the handle and check the reason.
uv_close(reinterpret_cast<uv_handle_t*>(uvHandle), static_cast<uv_close_cb>(onClose));
uv_close(reinterpret_cast<uv_handle_t*>(uvHandle), static_cast<uv_close_cb>(onCloseUdp));

MS_THROW_ERROR(
"uv_udp_bind() failed [transport:%s, ip:'%s', port:%" PRIu16 "]: %s",
Expand All @@ -449,7 +493,7 @@ namespace RTC
if (err != 0)
{
// If it failed, close the handle and check the reason.
uv_close(reinterpret_cast<uv_handle_t*>(uvHandle), static_cast<uv_close_cb>(onClose));
uv_close(reinterpret_cast<uv_handle_t*>(uvHandle), static_cast<uv_close_cb>(onCloseTcp));

MS_THROW_ERROR(
"uv_tcp_bind() failed [transport:%s, ip:'%s', port:%" PRIu16 "]: %s",
Expand All @@ -469,7 +513,7 @@ namespace RTC
if (err != 0)
{
// If it failed, close the handle and check the reason.
uv_close(reinterpret_cast<uv_handle_t*>(uvHandle), static_cast<uv_close_cb>(onClose));
uv_close(reinterpret_cast<uv_handle_t*>(uvHandle), static_cast<uv_close_cb>(onCloseTcp));

MS_THROW_ERROR(
"uv_listen() failed [transport:%s, ip:'%s', port:%" PRIu16 "]: %s",
Expand Down
6 changes: 3 additions & 3 deletions worker/src/handles/SignalsHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@ inline static void onSignal(uv_signal_t* handle, int signum)
static_cast<SignalsHandler*>(handle->data)->OnUvSignal(signum);
}

inline static void onClose(uv_handle_t* handle)
inline static void onCloseSignal(uv_handle_t* handle)
{
delete handle;
delete reinterpret_cast<uv_signal_t*>(handle);
}

/* Instance methods. */
Expand Down Expand Up @@ -44,7 +44,7 @@ void SignalsHandler::Close()

for (auto* uvHandle : this->uvHandles)
{
uv_close(reinterpret_cast<uv_handle_t*>(uvHandle), static_cast<uv_close_cb>(onClose));
uv_close(reinterpret_cast<uv_handle_t*>(uvHandle), static_cast<uv_close_cb>(onCloseSignal));
}
}

Expand Down
15 changes: 11 additions & 4 deletions worker/src/handles/TcpConnectionHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,16 @@ inline static void onWrite(uv_write_t* req, int status)
delete writeData;
}

inline static void onClose(uv_handle_t* handle)
// NOTE: We have different onCloseXxx() callbacks to avoid an ASAN warning by
// ensuring that we call `delete xxx` with same type as `new xxx` before.
inline static void onCloseTcp(uv_handle_t* handle)
{
delete handle;
delete reinterpret_cast<uv_tcp_t*>(handle);
}

inline static void onCloseShutdown(uv_handle_t* handle)
{
delete reinterpret_cast<uv_shutdown_t*>(handle);
}

inline static void onShutdown(uv_shutdown_t* req, int /*status*/)
Expand All @@ -52,7 +59,7 @@ inline static void onShutdown(uv_shutdown_t* req, int /*status*/)
delete req;

// Now do close the handle.
uv_close(reinterpret_cast<uv_handle_t*>(handle), static_cast<uv_close_cb>(onClose));
uv_close(reinterpret_cast<uv_handle_t*>(handle), static_cast<uv_close_cb>(onCloseShutdown));
}

/* Instance methods. */
Expand Down Expand Up @@ -114,7 +121,7 @@ void TcpConnectionHandler::Close()
// Otherwise directly close the socket.
else
{
uv_close(reinterpret_cast<uv_handle_t*>(this->uvHandle), static_cast<uv_close_cb>(onClose));
uv_close(reinterpret_cast<uv_handle_t*>(this->uvHandle), static_cast<uv_close_cb>(onCloseTcp));
}
}

Expand Down
10 changes: 5 additions & 5 deletions worker/src/handles/TcpServerHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,9 @@ inline static void onConnection(uv_stream_t* handle, int status)
server->OnUvConnection(status);
}

inline static void onClose(uv_handle_t* handle)
inline static void onCloseTcp(uv_handle_t* handle)
{
delete handle;
delete reinterpret_cast<uv_tcp_t*>(handle);
}

/* Instance methods. */
Expand All @@ -43,15 +43,15 @@ TcpServerHandler::TcpServerHandler(uv_tcp_t* uvHandle) : uvHandle(uvHandle)

if (err != 0)
{
uv_close(reinterpret_cast<uv_handle_t*>(this->uvHandle), static_cast<uv_close_cb>(onClose));
uv_close(reinterpret_cast<uv_handle_t*>(this->uvHandle), static_cast<uv_close_cb>(onCloseTcp));

MS_THROW_ERROR("uv_listen() failed: %s", uv_strerror(err));
}

// Set local address.
if (!SetLocalAddress())
{
uv_close(reinterpret_cast<uv_handle_t*>(this->uvHandle), static_cast<uv_close_cb>(onClose));
uv_close(reinterpret_cast<uv_handle_t*>(this->uvHandle), static_cast<uv_close_cb>(onCloseTcp));

MS_THROW_ERROR("error setting local IP and port");
}
Expand Down Expand Up @@ -84,7 +84,7 @@ void TcpServerHandler::Close()
delete connection;
}

uv_close(reinterpret_cast<uv_handle_t*>(this->uvHandle), static_cast<uv_close_cb>(onClose));
uv_close(reinterpret_cast<uv_handle_t*>(this->uvHandle), static_cast<uv_close_cb>(onCloseTcp));
}

void TcpServerHandler::Dump() const
Expand Down
6 changes: 3 additions & 3 deletions worker/src/handles/Timer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@ inline static void onTimer(uv_timer_t* handle)
static_cast<Timer*>(handle->data)->OnUvTimer();
}

inline static void onClose(uv_handle_t* handle)
inline static void onCloseTimer(uv_handle_t* handle)
{
delete handle;
delete reinterpret_cast<uv_timer_t*>(handle);
}

/* Instance methods. */
Expand Down Expand Up @@ -59,7 +59,7 @@ void Timer::Close()

this->closed = true;

uv_close(reinterpret_cast<uv_handle_t*>(this->uvHandle), static_cast<uv_close_cb>(onClose));
uv_close(reinterpret_cast<uv_handle_t*>(this->uvHandle), static_cast<uv_close_cb>(onCloseTimer));
}

void Timer::Start(uint64_t timeout, uint64_t repeat)
Expand Down
10 changes: 5 additions & 5 deletions worker/src/handles/UdpSocketHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,9 @@ inline static void onSend(uv_udp_send_t* req, int status)
delete sendData;
}

inline static void onClose(uv_handle_t* handle)
inline static void onCloseUdp(uv_handle_t* handle)
{
delete handle;
delete reinterpret_cast<uv_udp_t*>(handle);
}

/* Instance methods. */
Expand All @@ -66,15 +66,15 @@ UdpSocketHandler::UdpSocketHandler(uv_udp_t* uvHandle) : uvHandle(uvHandle)

if (err != 0)
{
uv_close(reinterpret_cast<uv_handle_t*>(this->uvHandle), static_cast<uv_close_cb>(onClose));
uv_close(reinterpret_cast<uv_handle_t*>(this->uvHandle), static_cast<uv_close_cb>(onCloseUdp));

MS_THROW_ERROR("uv_udp_recv_start() failed: %s", uv_strerror(err));
}

// Set local address.
if (!SetLocalAddress())
{
uv_close(reinterpret_cast<uv_handle_t*>(this->uvHandle), static_cast<uv_close_cb>(onClose));
uv_close(reinterpret_cast<uv_handle_t*>(this->uvHandle), static_cast<uv_close_cb>(onCloseUdp));

MS_THROW_ERROR("error setting local IP and port");
}
Expand Down Expand Up @@ -106,7 +106,7 @@ void UdpSocketHandler::Close()
if (err != 0)
MS_ABORT("uv_udp_recv_stop() failed: %s", uv_strerror(err));

uv_close(reinterpret_cast<uv_handle_t*>(this->uvHandle), static_cast<uv_close_cb>(onClose));
uv_close(reinterpret_cast<uv_handle_t*>(this->uvHandle), static_cast<uv_close_cb>(onCloseUdp));
}

void UdpSocketHandler::Dump() const
Expand Down
Loading

0 comments on commit 222ebd2

Please sign in to comment.