Fix and test heap allocation of PacketBuffers (#4632)
* Fix and test heap allocation of PacketBuffers
#### Problem
CHIP has to option to use heap-allocated packet buffers
(with `CHIP_SYSTEM_CONFIG_PACKETBUFFER_POOL_SIZE == 0`) but
this configuration has been neglected, and CHIP neither builds
nor runs in this configuration.
#### Summary of Changes
Build fixes:
- Default `CHIP_CONFIG_RMP_RETRANS_TABLE_SIZE` to 15 for heap case.
15 is the default packet buffer pool size in SystemConfig.h.
See also issue #4335 - Our definition of the CRMP retransmission
table size does not really make sense
Runtime fixes:
- Ensure echo server has enough buffer space for the response (and added
`CHECK_RETURN_VALUE` to `EnsureReservedSize()`).
- Added room for MAC in echo requester.
- Added room for MAC in `chipSendUnicast()`.
- Added parameters to `CloneData()` to request header and trailing
space.
- Fixed CHIPMem initialization for some tests.
- Fix tests that didn't allocate enough space.
Build and code changes to assist memory troubleshooting (these should have
no cost unless enabled):
- Added a build flag `chip_config_memory_debug_checks` and corresponding
preprocessor definition `CHIP_CONFIG_MEMORY_DEBUG_CHECKS` to enable extra
memory checks, at a performance cost. In particular, new functions
`Platform::MemoryDebugCheckPointer()` and `PacketBufferHandle::Check()`
operate when this is enabled.
- Added a build flag `chip_config_memory_debug_dmalloc` and corresponding
preprocessor definition `CHIP_CONFIG_MEMORY_DEBUG_DMALLOC` to use the
dmalloc debugging malloc library.
- Clarified conditions for the four configurations of PacketBuffer allocation.
- Replaced some PacketBuffer #define constants with scoped constexprs.
- Renamed `CHIP_SYSTEM_CONFIG_PACKETBUFFER_MAXALLOC` to
`CHIP_SYSTEM_CONFIG_PACKETBUFFER_POOL_SIZE` to remove the ambiguity of
what a ‘maximum allocation’ is.
Build and automated test changes:
- Made standalone builds default to heap allocation.
- Run a set of unit tests with dmalloc.
fixes #4395 - Heap packet buffer allocation is broken
fixes #4390 - ReliableMessageProtocol assumes a buffer pool
* review
* review 2
* fix messaging shutdown
* review 3
* fix tests
Co-authored-by: Justin Wood <woody@apple.com>diff --git a/src/app/tests/TestMessageDef.cpp b/src/app/tests/TestMessageDef.cpp
index 2425351..ff4a135 100644
--- a/src/app/tests/TestMessageDef.cpp
+++ b/src/app/tests/TestMessageDef.cpp
@@ -28,6 +28,7 @@
#include <app/MessageDef/ReadRequest.h>
#include <app/MessageDef/ReportData.h>
#include <core/CHIPTLVDebug.hpp>
+#include <support/CHIPMem.h>
#include <support/UnitTestRegistration.h>
#include <system/TLVPacketBufferBackingStore.h>
@@ -748,7 +749,7 @@
AttributePath::Builder attributePathBuilder;
chip::System::PacketBufferTLVWriter writer;
chip::System::PacketBufferTLVReader reader;
- writer.Init(chip::System::PacketBufferHandle::New(chip::System::kMaxPacketBufferSize));
+ writer.Init(chip::System::PacketBufferHandle::New(chip::System::PacketBuffer::kMaxSize));
attributePathBuilder.Init(&writer);
BuildAttributePath(apSuite, attributePathBuilder);
chip::System::PacketBufferHandle buf;
@@ -771,7 +772,7 @@
chip::System::PacketBufferTLVReader reader;
AttributePathList::Builder attributePathListBuilder;
- writer.Init(chip::System::PacketBufferHandle::New(chip::System::kMaxPacketBufferSize));
+ writer.Init(chip::System::PacketBufferHandle::New(chip::System::PacketBuffer::kMaxSize));
err = attributePathListBuilder.Init(&writer);
NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);
@@ -796,7 +797,7 @@
EventPath::Builder eventPathBuilder;
chip::System::PacketBufferTLVWriter writer;
chip::System::PacketBufferTLVReader reader;
- writer.Init(chip::System::PacketBufferHandle::New(chip::System::kMaxPacketBufferSize));
+ writer.Init(chip::System::PacketBufferHandle::New(chip::System::PacketBuffer::kMaxSize));
eventPathBuilder.Init(&writer);
BuildEventPath(apSuite, eventPathBuilder);
chip::System::PacketBufferHandle buf;
@@ -820,7 +821,7 @@
chip::System::PacketBufferTLVReader reader;
EventPathList::Builder eventPathListBuilder;
- writer.Init(chip::System::PacketBufferHandle::New(chip::System::kMaxPacketBufferSize));
+ writer.Init(chip::System::PacketBufferHandle::New(chip::System::PacketBuffer::kMaxSize));
err = eventPathListBuilder.Init(&writer);
NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);
@@ -844,7 +845,7 @@
chip::System::PacketBufferTLVWriter writer;
chip::System::PacketBufferTLVReader reader;
CommandPath::Builder commandPathBuilder;
- writer.Init(chip::System::PacketBufferHandle::New(chip::System::kMaxPacketBufferSize));
+ writer.Init(chip::System::PacketBufferHandle::New(chip::System::PacketBuffer::kMaxSize));
err = commandPathBuilder.Init(&writer);
NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);
@@ -870,7 +871,7 @@
EventDataElement::Parser eventDataElementParser;
chip::System::PacketBufferTLVWriter writer;
chip::System::PacketBufferTLVReader reader;
- writer.Init(chip::System::PacketBufferHandle::New(chip::System::kMaxPacketBufferSize));
+ writer.Init(chip::System::PacketBufferHandle::New(chip::System::PacketBuffer::kMaxSize));
eventDataElementBuilder.Init(&writer);
BuildEventDataElement(apSuite, eventDataElementBuilder);
chip::System::PacketBufferHandle buf;
@@ -893,7 +894,7 @@
chip::System::PacketBufferTLVWriter writer;
chip::System::PacketBufferTLVReader reader;
EventList::Builder eventListBuilder;
- writer.Init(chip::System::PacketBufferHandle::New(chip::System::kMaxPacketBufferSize));
+ writer.Init(chip::System::PacketBufferHandle::New(chip::System::PacketBuffer::kMaxSize));
eventListBuilder.Init(&writer);
BuildEventList(apSuite, eventListBuilder);
chip::System::PacketBufferHandle buf;
@@ -915,7 +916,7 @@
StatusElement::Parser statusElementParser;
chip::System::PacketBufferTLVWriter writer;
chip::System::PacketBufferTLVReader reader;
- writer.Init(chip::System::PacketBufferHandle::New(chip::System::kMaxPacketBufferSize));
+ writer.Init(chip::System::PacketBufferHandle::New(chip::System::PacketBuffer::kMaxSize));
statusElementBuilder.Init(&writer);
BuildStatusElement(apSuite, statusElementBuilder);
chip::System::PacketBufferHandle buf;
@@ -939,7 +940,7 @@
AttributeStatusElement::Parser attributeStatusElementParser;
chip::System::PacketBufferTLVWriter writer;
chip::System::PacketBufferTLVReader reader;
- writer.Init(chip::System::PacketBufferHandle::New(chip::System::kMaxPacketBufferSize));
+ writer.Init(chip::System::PacketBufferHandle::New(chip::System::PacketBuffer::kMaxSize));
attributeStatusElementBuilder.Init(&writer);
BuildAttributeStatusElement(apSuite, attributeStatusElementBuilder);
chip::System::PacketBufferHandle buf;
@@ -961,7 +962,7 @@
CHIP_ERROR err = CHIP_NO_ERROR;
chip::System::PacketBufferTLVWriter writer;
chip::System::PacketBufferTLVReader reader;
- writer.Init(chip::System::PacketBufferHandle::New(chip::System::kMaxPacketBufferSize));
+ writer.Init(chip::System::PacketBufferHandle::New(chip::System::PacketBuffer::kMaxSize));
AttributeStatusList::Builder attributeStatusListBuilder;
err = attributeStatusListBuilder.Init(&writer);
NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);
@@ -985,7 +986,7 @@
AttributeDataElement::Parser attributeDataElementParser;
chip::System::PacketBufferTLVWriter writer;
chip::System::PacketBufferTLVReader reader;
- writer.Init(chip::System::PacketBufferHandle::New(chip::System::kMaxPacketBufferSize));
+ writer.Init(chip::System::PacketBufferHandle::New(chip::System::PacketBuffer::kMaxSize));
attributeDataElementBuilder.Init(&writer);
BuildAttributeDataElement(apSuite, attributeDataElementBuilder);
chip::System::PacketBufferHandle buf;
@@ -1007,7 +1008,7 @@
CHIP_ERROR err = CHIP_NO_ERROR;
chip::System::PacketBufferTLVWriter writer;
chip::System::PacketBufferTLVReader reader;
- writer.Init(chip::System::PacketBufferHandle::New(chip::System::kMaxPacketBufferSize));
+ writer.Init(chip::System::PacketBufferHandle::New(chip::System::PacketBuffer::kMaxSize));
AttributeDataList::Builder attributeDataListBuilder;
attributeDataListBuilder.Init(&writer);
BuildAttributeDataList(apSuite, attributeDataListBuilder);
@@ -1028,7 +1029,7 @@
CHIP_ERROR err = CHIP_NO_ERROR;
chip::System::PacketBufferTLVWriter writer;
chip::System::PacketBufferTLVReader reader;
- writer.Init(chip::System::PacketBufferHandle::New(chip::System::kMaxPacketBufferSize));
+ writer.Init(chip::System::PacketBufferHandle::New(chip::System::PacketBuffer::kMaxSize));
AttributeDataVersionList::Builder attributeDataVersionListBuilder;
attributeDataVersionListBuilder.Init(&writer);
BuildAttributeDataVersionList(apSuite, attributeDataVersionListBuilder);
@@ -1051,7 +1052,7 @@
CommandDataElement::Parser commandDataElementParser;
chip::System::PacketBufferTLVWriter writer;
chip::System::PacketBufferTLVReader reader;
- writer.Init(chip::System::PacketBufferHandle::New(chip::System::kMaxPacketBufferSize));
+ writer.Init(chip::System::PacketBufferHandle::New(chip::System::PacketBuffer::kMaxSize));
commandDataElementBuilder.Init(&writer);
BuildCommandDataElement(apSuite, commandDataElementBuilder);
chip::System::PacketBufferHandle buf;
@@ -1074,7 +1075,7 @@
chip::System::PacketBufferTLVWriter writer;
chip::System::PacketBufferTLVReader reader;
CommandList::Builder commandListBuilder;
- writer.Init(chip::System::PacketBufferHandle::New(chip::System::kMaxPacketBufferSize));
+ writer.Init(chip::System::PacketBufferHandle::New(chip::System::PacketBuffer::kMaxSize));
commandListBuilder.Init(&writer);
BuildCommandList(apSuite, commandListBuilder);
chip::System::PacketBufferHandle buf;
@@ -1094,7 +1095,7 @@
CHIP_ERROR err = CHIP_NO_ERROR;
chip::System::PacketBufferTLVWriter writer;
chip::System::PacketBufferTLVReader reader;
- writer.Init(chip::System::PacketBufferHandle::New(chip::System::kMaxPacketBufferSize));
+ writer.Init(chip::System::PacketBufferHandle::New(chip::System::PacketBuffer::kMaxSize));
BuildReportData(apSuite, writer);
chip::System::PacketBufferHandle buf;
err = writer.Finalize(&buf);
@@ -1113,7 +1114,7 @@
CHIP_ERROR err = CHIP_NO_ERROR;
chip::System::PacketBufferTLVWriter writer;
chip::System::PacketBufferTLVReader reader;
- writer.Init(chip::System::PacketBufferHandle::New(chip::System::kMaxPacketBufferSize));
+ writer.Init(chip::System::PacketBufferHandle::New(chip::System::PacketBuffer::kMaxSize));
BuildInvokeCommand(apSuite, writer);
chip::System::PacketBufferHandle buf;
err = writer.Finalize(&buf);
@@ -1132,7 +1133,7 @@
CHIP_ERROR err = CHIP_NO_ERROR;
chip::System::PacketBufferTLVWriter writer;
chip::System::PacketBufferTLVReader reader;
- writer.Init(chip::System::PacketBufferHandle::New(chip::System::kMaxPacketBufferSize));
+ writer.Init(chip::System::PacketBufferHandle::New(chip::System::PacketBuffer::kMaxSize));
BuildReadRequest(apSuite, writer);
chip::System::PacketBufferHandle buf;
err = writer.Finalize(&buf);
@@ -1176,6 +1177,26 @@
// clang-format on
} // namespace
+/**
+ * Set up the test suite.
+ */
+static int TestSetup(void * inContext)
+{
+ CHIP_ERROR error = chip::Platform::MemoryInit();
+ if (error != CHIP_NO_ERROR)
+ return FAILURE;
+ return SUCCESS;
+}
+
+/**
+ * Tear down the test suite.
+ */
+static int TestTeardown(void * inContext)
+{
+ chip::Platform::MemoryShutdown();
+ return SUCCESS;
+}
+
int TestMessageDef()
{
// clang-format off
@@ -1183,8 +1204,8 @@
{
"MessageDef",
&sTests[0],
- nullptr,
- nullptr
+ TestSetup,
+ TestTeardown,
};
// clang-format on
