esp-idf: Ringbuffer timeouts in certain cases (IDFGH-5624)
I was experiencing some weird issues with regards to the Ringbuffer api, so I wrote a few tests to reproduce my problem. Both of the following tests fail on the “Timeout” assert. The issue is sensitive to the buffer size, the data size and I also had to insert data with different sizes.
Original, more complex examples - Click to expand!
#include "unity.h"
#include "freertos/FreeRTOS.h"
#include "freertos/ringbuf.h"
#include <stdio.h>
#include <string.h>
TEST_CASE("ringbuf xRingbufferSend problem case", "[ringbuf]") {
RingbufHandle_t ringbuffer = xRingbufferCreate(200, RINGBUF_TYPE_NOSPLIT);
uint8_t data[9] = {3, 4, 5, 6, 7, 8, 9, 10, 11};
for (size_t size = 8; size <= sizeof(data); size++) {
for (size_t i = 0; i < 40; i++) {
// Make space by removing records
while (xRingbufferGetCurFreeSize(ringbuffer) < size) {
void* item = xRingbufferReceive(ringbuffer, NULL, 0);
if (item == NULL) {
TEST_ASSERT(xRingbufferGetCurFreeSize(ringbuffer) ==
xRingbufferGetMaxItemSize(ringbuffer));
break;
}
// Removed record has the expected data
TEST_ASSERT_EQUAL_MEMORY(item, data, size);
vRingbufferReturnItem(ringbuffer, item);
}
TEST_ASSERT(xRingbufferGetCurFreeSize(ringbuffer) >= size);
if (xRingbufferSend(ringbuffer, data, size, 0) != pdTRUE) {
printf("Failed to acquire memory in event buffer. Required: %u Max: %u Free: %u\n",
size, xRingbufferGetMaxItemSize(ringbuffer),
xRingbufferGetCurFreeSize(ringbuffer));
// If there is enough space, xRingbufferSend timed out.
TEST_ASSERT_MESSAGE(xRingbufferGetCurFreeSize(ringbuffer) < size,
"xRingbufferSend Timeout");
}
}
// Flush buffer
void* item;
do {
item = xRingbufferReceive(ringbuffer, NULL, 0);
if (item) {
// Record has expected data
TEST_ASSERT_EQUAL_MEMORY(item, data, size);
vRingbufferReturnItem(ringbuffer, item);
}
} while (item != NULL);
}
vRingbufferDelete(ringbuffer);
}
TEST_CASE("ringbuf xRingbufferSendAcquire problem case", "[ringbuf]") {
RingbufHandle_t ringbuffer = xRingbufferCreate(200, RINGBUF_TYPE_NOSPLIT);
uint8_t data[9] = {3, 4, 5, 6, 7, 8, 9, 10, 11};
for (size_t size = 8; size <= sizeof(data); size++) {
for (size_t i = 0; i < 40; i++) {
// Make space by removing records
while (xRingbufferGetCurFreeSize(ringbuffer) < size) {
void* item = xRingbufferReceive(ringbuffer, NULL, 0);
if (item == NULL) {
TEST_ASSERT(xRingbufferGetCurFreeSize(ringbuffer) ==
xRingbufferGetMaxItemSize(ringbuffer));
break;
}
// Removed record has the expected data
TEST_ASSERT_EQUAL_MEMORY(item, data, size);
vRingbufferReturnItem(ringbuffer, item);
}
TEST_ASSERT(xRingbufferGetCurFreeSize(ringbuffer) >= size);
void* pItem;
if (xRingbufferSendAcquire(ringbuffer, &pItem, size, 0) == pdTRUE) {
memcpy(pItem, data, size);
xRingbufferSendComplete(ringbuffer, pItem);
} else {
printf("Failed to acquire memory in event buffer. Required: %u Max: %u Free: %u\n",
size, xRingbufferGetMaxItemSize(ringbuffer),
xRingbufferGetCurFreeSize(ringbuffer));
// If there is enough space, xRingbufferSendAcquire timed out.
TEST_ASSERT_MESSAGE(xRingbufferGetCurFreeSize(ringbuffer) < size,
"xRingbufferSendAcquire Timeout");
}
}
// Flush buffer
void* item;
do {
item = xRingbufferReceive(ringbuffer, NULL, 0);
if (item) {
// Record has expected data
TEST_ASSERT_EQUAL_MEMORY(item, data, size);
vRingbufferReturnItem(ringbuffer, item);
}
} while (item != NULL);
}
vRingbufferDelete(ringbuffer);
}
Edit: I’ve made a simpler test case:
TEST_CASE("simple case", "[ringbuf]") {
uint8_t data[9] = {3, 4, 5, 6, 7, 8, 9, 10, 11};
RingbufHandle_t ringbuffer = xRingbufferCreate(80, RINGBUF_TYPE_NOSPLIT);
TEST_ASSERT(xRingbufferSend(ringbuffer, data, 8, 0));
TEST_ASSERT(xRingbufferSend(ringbuffer, data, 8, 0));
TEST_ASSERT(xRingbufferSend(ringbuffer, data, 8, 0));
TEST_ASSERT(xRingbufferSend(ringbuffer, data, 8, 0));
void* item;
item = xRingbufferReceive(ringbuffer, NULL, 0);
TEST_ASSERT(item != NULL);
vRingbufferReturnItem(ringbuffer, item);
item = xRingbufferReceive(ringbuffer, NULL, 0);
TEST_ASSERT(item != NULL);
vRingbufferReturnItem(ringbuffer, item);
item = xRingbufferReceive(ringbuffer, NULL, 0);
TEST_ASSERT(item != NULL);
vRingbufferReturnItem(ringbuffer, item);
TEST_ASSERT(xRingbufferGetCurFreeSize(ringbuffer) >= 9);
TEST_ASSERT(xRingbufferSend(ringbuffer, data, 10, 0));
item = xRingbufferReceive(ringbuffer, NULL, 0);
TEST_ASSERT(item != NULL);
vRingbufferReturnItem(ringbuffer, item);
TEST_ASSERT(xRingbufferGetCurFreeSize(ringbuffer) >= 9);
TEST_ASSERT(xRingbufferSend(ringbuffer, data, 9, 0));
TEST_ASSERT(xRingbufferGetCurFreeSize(ringbuffer) >= 9);
TEST_ASSERT(xRingbufferSend(ringbuffer, data, 9, 0));
TEST_ASSERT(xRingbufferGetCurFreeSize(ringbuffer) >= 9);
TEST_ASSERT(xRingbufferSend(ringbuffer, data, 9, 0));
vRingbufferDelete(ringbuffer);
}
About this issue
- Original URL
- State: closed
- Created 3 years ago
- Comments: 18 (12 by maintainers)
Commits related to this issue
- Ringbuf [nosplit] bugfixes Fix tracking free space. Fix a number of potential underflow issues, as well as a few incorrect comments. Don't round up data size for the last chunk. Fixes #7344 — committed to bugadani/esp-idf by bugadani 3 years ago
- Ringbuf [nosplit] bugfixes Fix tracking free space. Fix a number of potential underflow issues, as well as a few incorrect comments. Don't round up data size for the last chunk. Fixes #7344 — committed to bugadani/esp-idf by bugadani 3 years ago
- Ringbuf [nosplit] bugfixes Fix tracking free space. Fix a number of potential underflow issues, as well as a few incorrect comments. Don't round up data size for the last chunk. Fixes #7344 — committed to bugadani/esp-idf by bugadani 3 years ago
- ringbuf: Fix bug where comparision between a signed and unsigned operand resulted in incorrect free size for no-split/allow-split buffers This commit fixes a bug in no-split and allow-split ring buff... — committed to espressif/esp-idf by sudeep-mohanty 3 years ago
- ringbuf: Fix bug where comparision between a signed and unsigned operand resulted in incorrect free size for no-split/allow-split buffers This commit fixes a bug in no-split and allow-split ring buff... — committed to espressif/esp-idf by sudeep-mohanty 3 years ago
- ringbuf: Fix bug where comparision between a signed and unsigned operand resulted in incorrect free size for no-split/allow-split buffers This commit fixes a bug in no-split and allow-split ring buff... — committed to espressif/esp-idf by sudeep-mohanty 3 years ago
- ringbuf: Fix bug where comparision between a signed and unsigned operand resulted in incorrect free size for no-split/allow-split buffers This commit fixes a bug in no-split and allow-split ring buff... — committed to espressif/esp-idf by sudeep-mohanty 3 years ago
- ringbuf: Fix bug where comparision between a signed and unsigned operand resulted in incorrect free size for no-split/allow-split buffers This commit fixes a bug in no-split and allow-split ring buff... — committed to espressif/esp-idf by sudeep-mohanty 3 years ago
@bugadani I’ve confirmed that this indeed a bug. The source of the issue is in
xRingbufferReceive(). The internal implementation (i.e.,prvGetItemDefault()) forgets to wrap the read pointer around a dummy data block. This results in the free pointer also being corrupted when the item is returned.I’ll push a fix for this issue shortly. For now, to work around this issue you can replace the following lines of
prvGetItemDefault()inringbuf.cas such:From…
To