mirror of
https://github.com/wpilibsuite/allwpilib
synced 2026-06-19 00:41:43 +00:00
[ntcore] Fix unbounded memory size tracker (#8929)
If a `ClientMessageQueue` is at a
memory limit, `ClientMessageQueueImpl::ClientSetValue` will still
increment `m_valueSize.size` even though no element has actually been
actually been added.
This quickly results in things like topics completely shutting down
under high load, as no new elements can be added as `m_valueSize.size`
explodes to infinity unless `ClearQueue()` is explicitly called. You can
see this pretty easily with the following code, be it in sim or on a
robot:
```cpp
#include "wpi/framework/TimedRobot.hpp"
#include "wpi/smartdashboard/SmartDashboard.hpp"
class Robot : public wpi::TimedRobot {
public:
// ...
uint64_t value{0};
/**
* This function is called periodically during all modes
*/
void RobotPeriodic() override {
for (int i = 0; i < 1'000'000; i++) {
value += 1;
wpi::SmartDashboard::PutNumber("value", value);
}
}
}
```
Connecting via your favorite dashboard quickly reveals that no new
values get added after a while. However, with this patch implemented,
values do still continue to increment as updates gradually get serviced.
This commit is contained in:
@@ -106,14 +106,15 @@ class ClientMessageQueueImpl final : public ClientMessageHandler,
|
|||||||
void ClientSetValue(int pubuid, const Value& value) final {
|
void ClientSetValue(int pubuid, const Value& value) final {
|
||||||
std::scoped_lock lock{m_mutex};
|
std::scoped_lock lock{m_mutex};
|
||||||
if constexpr (MaxValueSize != 0) {
|
if constexpr (MaxValueSize != 0) {
|
||||||
m_valueSize.size += sizeof(ClientMessage) + value.size();
|
size_t addedSize = sizeof(ClientMessage) + value.size();
|
||||||
if (m_valueSize.size > MaxValueSize) {
|
if (m_valueSize.size + addedSize > MaxValueSize) {
|
||||||
if (!m_valueSize.errored) {
|
if (!m_valueSize.errored) {
|
||||||
WPI_ERROR(m_logger, "NT: dropping value set due to memory limits");
|
WPI_ERROR(m_logger, "NT: dropping value set due to memory limits");
|
||||||
m_valueSize.errored = true;
|
m_valueSize.errored = true;
|
||||||
}
|
}
|
||||||
return; // avoid potential out of memory
|
return; // avoid potential out of memory
|
||||||
}
|
}
|
||||||
|
m_valueSize.size += addedSize;
|
||||||
}
|
}
|
||||||
m_queue.enqueue(ClientMessage{ClientValueMsg{pubuid, value}});
|
m_queue.enqueue(ClientMessage{ClientValueMsg{pubuid, value}});
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user