[ntcore] Fix crash on disconnect (#5788)

The object was being destroyed due to the error handling path.
Instead, defer to the next loop cycle using a one-shot timer.

Properly handle error return values from Send functions.

Fix UB in accessing one past the end of a vector.
This commit is contained in:
Peter Johnson
2023-10-19 14:36:22 -07:00
committed by GitHub
parent 85147bf69e
commit 7c6fe56cf2
4 changed files with 26 additions and 3 deletions

View File

@@ -242,7 +242,11 @@ void NetworkClient3::TcpConnected(uv::Tcp& tcp) {
tcp.error.connect([this, &tcp](uv::Error err) {
DEBUG3("NT3 TCP error {}", err.str());
if (!tcp.IsLoopClosing()) {
DoDisconnect(err.str());
// we could be in the middle of sending data, so defer disconnect
uv::Timer::SingleShot(m_loop, uv::Timer::Time{0},
[this, reason = std::string{err.str()}] {
DoDisconnect(reason);
});
}
});
tcp.end.connect([this, &tcp] {
@@ -412,7 +416,10 @@ void NetworkClient::WsConnected(wpi::WebSocket& ws, uv::Tcp& tcp,
m_clientImpl->SendInitial();
ws.closed.connect([this, &ws](uint16_t, std::string_view reason) {
if (!ws.GetStream().IsLoopClosing()) {
DoDisconnect(reason);
// we could be in the middle of sending data, so defer disconnect
uv::Timer::SingleShot(
m_loop, uv::Timer::Time{0},
[this, reason = std::string{reason}] { DoDisconnect(reason); });
}
});
ws.text.connect([this](std::string_view data, bool) {

View File

@@ -280,9 +280,15 @@ void NetworkOutgoingQueue<MessageType>::SendOutgoing(uint64_t curTimeMs,
});
}
}
if (unsent < 0) {
return; // error
}
if (unsent == 0) {
// finish writing any partial buffers
unsent = m_wire.Flush();
if (unsent < 0) {
return; // error
}
}
int delta = it - msgs.begin() - unsent;
for (auto&& msg : std::span{msgs}.subspan(0, delta)) {

View File

@@ -524,6 +524,9 @@ void ServerImpl::ClientData4::SendAnnounce(TopicData* topic,
WireEncodeAnnounce(os, topic->name, topic->id, topic->typeStr,
topic->properties, pubuid);
});
if (unsent < 0) {
return; // error
}
if (unsent == 0 && m_wire.Flush() == 0) {
return;
}
@@ -544,6 +547,9 @@ void ServerImpl::ClientData4::SendUnannounce(TopicData* topic) {
if (m_local) {
int unsent = m_wire.WriteText(
[&](auto& os) { WireEncodeUnannounce(os, topic->name, topic->id); });
if (unsent < 0) {
return; // error
}
if (unsent == 0 && m_wire.Flush() == 0) {
return;
}
@@ -565,6 +571,9 @@ void ServerImpl::ClientData4::SendPropertiesUpdate(TopicData* topic,
int unsent = m_wire.WriteText([&](auto& os) {
WireEncodePropertiesUpdate(os, topic->name, update, ack);
});
if (unsent < 0) {
return; // error
}
if (unsent == 0 && m_wire.Flush() == 0) {
return;
}

View File

@@ -186,7 +186,8 @@ int WebSocketConnection::Flush() {
m_ws_frames.reserve(m_frames.size());
for (auto&& frame : m_frames) {
m_ws_frames.emplace_back(
frame.opcode, std::span{&m_bufs[frame.start], &m_bufs[frame.end]});
frame.opcode,
std::span{m_bufs}.subspan(frame.start, frame.end - frame.start));
}
auto unsentFrames = m_ws.TrySendFrames(