C++ wrapper for FreeRTOS blocking queue that can handle complex objects

Question

The FreeRTOS queue is a convenient way for intertask communication on FreeRTOS. But it is written in pain C and therefore it feels a little uncomfortable for C++ programmers. But the main problem is, that is can only handle POD objects.

I found some C++ wrappers for the queue but they all only work with POD classes. So I tried to create my own wrapper. It creates a deep copy of the object and then deallocates it without calling the destructor. This leaves all the heap data intact and when I pop the object it can continue using it.

Anyway, this works fine now but I'm not that confident with it. So i wonder if this works as I expect or just by accident. Is this still defined behavior? What could possibly go wrong?

To make things clearer, here an example: std::string usually has a sizeof 32 byte. These 32 bytes will be copied by xQueueSend, this works fine. But std::string holds a pointer a char* which contains the string data. This data will not be copied. When I give a pointer to the string to xQueueSend, the 32 byte will be copied but not the char array. Would the original std::string get deleted, it would also delete the char array and the data pointer of the copy in the Queue would be dangling. So I create a copy of the std::string with new but then don't delete it. This way the char array will not be deleted. To clean up the 32 bytes the new call has created, I call deallocate. This only deletes the 32 bytes, but not the char array. Reading from the Queue works the other way around. I first create a std::string, but without calling the constructor. This way, I can be sure the it doesn't allocate any additional heap memory. Now I call xQueueReceive with the new generated uninitialized memory. This copies the 32 byte from the string into the new string pointer. Now I have the fully initialized std::string with valid pointer to the char array.

/* This is a C++ Queue wrapper for FreeRTOS queues.
* It uses std::allocator to allocate memory for the queue items.
* The stack data of the objects is copied to the queue but the
* heap data will remain and will be reused when the object is
* popped from the queue.
*/
#pragma once
#include <freertos/FreeRTOS.h>
#include <freertos/queue.h>
#include <memory>
namespace {
 TickType_t ticksToMs(int ms) {
 return ms >= 0 ? ms / portTICK_PERIOD_MS : portMAX_DELAY;;
 }
}
template <typename T>
class Queue {
public:
 Queue(int size) :
 _queue{xQueueCreate(size, sizeof(T))} {
 assert(_queue);
 }
 ~Queue() {
 vQueueDelete(_queue);
 }
 bool push(const T &data, int timeout = -1) const {
 T *copy = new T{data};
 const BaseType_t ret = xQueueSend(_queue, static_cast<void*>(copy), msToTicks(timeout));
 _allocator.deallocate(copy, 1);
 return ret == pdTRUE;
 }
 T pop(int timeout = -1) {
 std::unique_ptr<T> data{_allocator.allocate(1)};
 const BaseType_t ret = xQueueReceive(_queue, static_cast<void*>(data.get()), msToTicks(timeout));
 if (ret == pdTRUE)
 return std::move(*data.get());
 return T{};
 }
private:
 struct Deallocator {
 void operator()(T* t) {
 _allocator.deallocate(t, 1); 
 }
 };
 QueueHandle_t _queue;
 static std::allocator<T> _allocator;
};

• Fe2O3: TickType_t ticksToMs(int ms) I'm really confused about what goes in, and what comes out of this function. Am I having a stroke??!!
• Mr. Clear: @Fe2O3 You are right, this might be the wrong function. For now I only used infinite blocking. Also, on my ESP32, the ticks per ms is 1, so this function does nothing at all.
• Fe2O3: I'm not going to post a review ('cuz I'm a masochist who thrives on "C pain"), but... both empty() and freeSpace() are ambiguous function names. Are they queries or are they actions with consequences? Using the handle 'Mr. Clear', you're not living up to it (imho)...
• indi: "It creates a deep copy of the object and then deallocates it without calling the destructor. This leaves all the heap data intact and when I pop the object it can continue using it." 😱 "Is this still defined behavior?" Good grief NO!!! "What could possibly go wrong?" Where to even begin? How about a situation where the standard allocator reuses the memory you freed, not realizing that you think there is a still a valid object sitting there? How about a situation where the system releases the now supposedly unused memory page, meaning you lose access to the object forever? And so on....
• indi: This is not a debate. You asked two questions: "Is this still defined behavior? What could possibly go wrong?" I gave the answers: no, and two ways it could go wrong. There are many more, and no, the fact that the data you want is maybe stored somewhere else does not make the problems go away. I am not going to go over every hypothetical way this could possibly blow up in your face while you keep trying to skirt around the various catastrophes. You have the answer: it is UB; that means anything can happen, like time travel optimizations that retroactively void all your assumptions.

Answer 1

Dealing properly with non-POD types

I found some C++ wrappers for the queue but they all only work with POD classes. So I tried to create my own wrapper. It creates a deep copy of the object and then deallocates it without calling the destructor.

As already mentioned, this is wrong. Consider a type whose behavior depends on whether it was moved in memory or not:

class Foo {
 Foo* self;
public:
 Foo(): self{this} {}
 bool check() { return this == self; }
};

Now consider what happens if you push it to and immediately pop it from a queue:

Foo *copy = new Foo;
assert(copy->check() == true);
xQueueSend(_queue, copy, ...);
_allocator.deallocate(copy, 1);
std::unique_ptr<Foo> data{_allocator.allocate(1)};
xQueueReceive(_queue, data.get(), ...);
assert(copy->check() == true); // FAILS!

This looks like a silly example, but there are types that work like this. They are safe to use if you let C++ copy or move them, but not if you use the equivalent of a C mempcy().

There are several options to deal with non-POD types properly though:

Allow only trivially copyable types

You can check if a type is trivially copyable (ie, with something like memcpy()) by using std::is_trivially_copyable in a static_assert() or requires clause.

Store pointers in the queue

Since you are already allocating memory for copy and data, why not just keep that memory around and store a pointer to that in the queue?

template <typename T>
class Queue {
public:
 Queue(int size) :
 _queue{xQueueCreate(size, sizeof(T*))} {
 assert(_queue);
 }
 bool push(const T &data, int timeout = -1) const {
 T *copy = new T{data};
 const BaseType_t ret = xQueueSend(_queue, &copy, msToTicks(timeout));
 if (ret == pdFALSE) {
 delete copy;
 }
 return ret == pdTRUE;
 }
 T pop(int timeout = -1) {
 T* copy;
 const BaseType_t ret = xQueueReceive(_queue, &copy, msToTicks(timeout));
 if (ret == pdTRUE) {
 T data = std::move(*data);
 delete copy;
 return data;
 }
 return T{};
 }
 ...
}

The above example copies data in push() and moves it in pop(), so it only works if T is both copyable and movable. You could add a std::move() to push() so it no longer copies. But you could also consider having push() take a T* and pop() return a T*, so your Queue no longer has to worry about memory management, and since it then doesn't copy or move anything, it will work with types that are both non-copyable and non-movable.

Unnecessary use of static_cast<void*>

C++ allows casting pointers to void* without needing a static_cast<>. The reverse is not true.

• Mr. Clear: Thank you. Now my only remaining question is why I have implemented such a cumbersome solution.
• Mr. Clear: There is still a problem. In push, the copy itself will not be deleted. In pop, copy is uninitialized, but xQueueReceive will try to write to the location it points on.
• G. Sliepen: @Mr.Clear Copying a pointer and sending/receiving a pointer to/from the queue is safe. That's what my example code does: copy is a T*, and by passing &copy to xQueueSend() and xQueueReceive(), it just stores the copy, not the T it points to.
• G. Sliepen: I meant, it just stores the pointer, not the data it points to.

Answer 2

FreeRTOS Queues handle memory management for you. The xQueueSend function stores objects by copy not not reference. Hence, you do not need manual memory management via new or std::allocate calls. This would mean something like this is sufficient.

bool push(const T &data, int timeout = -1) const {
 return xQueueSend(_queue, static_cast<void*>(copy), msToTicks(timeout)) == pdTRUE;
}
 std::optional<T> pop(int timeout = -1) {
 T t;
 const BaseType_t ret = xQueueReceive(_queue, static_cast<void*>(data.get()), msToTicks(timeout));
 if (ret == pdTRUE)
 return t;
 return {};
}

However, this also means that non-plain old datatypes classes are not compatible as you do not invoke the copy/move constructors. C++ does its dynamic memory management via calls to new/delete or std::allocate/std::deallocate. FreeRTOS does not. So, there is no way to wrap the freertos queue in C++ to support all C++ types without invoking UB. So, I would suggest a check via type_traits to constraint the T to pods. Something like this.

static_assert(std::is_pod_v<T>, "Queue only accepts plain old data_type");

I will further add move constructor and assignment operator to the queue. Adding copy constructor is impossible here without manually adding sync. But at the point, we pretty much lose main benifit of using FreeRTOS queue. So, I would explicitly delete those operations.

• G. Sliepen: Note that std::is_pod is deprecated since C++20, and you should use std::is_trivially_copyable in this case. Your code also doesn't compile; there's no copy in push() and no data in pop().
• indi: There is also no std::allocate or std::deallocate. 🤷🏼