Polymorphism and Queues

Kernel
1

Suppose that I have a class Request that has two derived classes WriteRequest and ReadRequest. The class holds a reference to it’s type through a type member as follows:

enum RequestType {
GENERIC_REQUEST = 0,
WRITE_REQUEST = 1,
READ_REQUEST = 2
};

class Request {
RequestType type = GENERIC_REQUEST;
};

class ReadRequest : public Request {
RequestType type = READ_REQUEST;
};

class WriteRequest : public Request {
RequestType type = WRITE_REQUEST;
};

Then, I have a Queue initialized as:

xQueueCreate(rqueue, Request, 20)

on which a task writes as follows:

void TaskSend(void* parameter) {
for (;;) {
// user-interaction logic

if (USER_INPUT == 'R') {
ReadRequest request;
xQueueSend(rqueue, &request);
}
else if (USER_INPUT == 'W') {
WriteRequest request;
xQueueSend(rqueue, &request);
};
}
}

Finally, I have a tasks that receives from the queue when available

void TaskReceive(void* parameter) {
Request request;

for (;;) {
xQueueReceive(rqueue, &request, pdMAX_DELAY);

if (request.type == READ_REQUEST)
cout << "Read" << endl;
else if (request.type == WRITE_REQUEST)
cout << "Write" << endl;
else
cout << "Generic" << endl;
};
}

I assumed that since the object passed to the queue is of type Read or Write, the received object should retain the .type member. However, the output is always “Generic”, I assume because the object is declared as a generic Request object before the for loop. It seems that xQueueSend/xQueueReceive fails to copy the object properties in the received request object.

Could someone explain me why this happens?

2

Your example is broken or is it pseudo code ?
You mean:
rqueue = xQueueCreate(20, sizeof(Request));
to send objects by value (copies) ?
Or do you want to transfer objects by reference/pointers ?
Then it’s:
rqueue = xQueueCreate(20, sizeof(Request*));
BUT: You can’t send objects allocated on stack by reference/pointer. It get’s a dangling reference/pointer when leaving the scope.

3

Yep, sorry, it is pseudo-code because I wrote the question on the go and I didn’t have the code with myself.

However, I want to pass the object by value, because the object passed can potentially be destroyed before getting handled.

I think that the problem here could lie in the container in which to parse the content of the queue with xQueueReceive. If I define the container with another type, no matter what I send, the type of the received variable is always equal to the one I have originally defined for the container (in this case, the parent class).

4

As usual: DO check all the return values of all API calls. Maybe your receive fails, so the contents of your buffer may be undefined.

5

Transfer by value should work. In fact queue send/receive memcpy’s the (binary) object in/out the queue. Means the object including the type member should be copied seamlessly in and out the queue. Defining the base class object outside the for loop is ok. The derived classes just have different constructors for the type member.
Try to trace it (the object before the send and the memory after the receive) with the debugger. Or double check your real code where the bug is laughing at you :wink:

6

I looked around a little bit. I have still to figure out a solution, but maybe this could flash an idea to someone more expert than me:

  • Internally, xQueueSend() calls memcopy. This would lead me to think that xQueueSend cannot handle complex (non-trivially-copiable) objects. In my case, since the original Request class is polymorphic (because, apart from the type, I have virtual member functions overridden in the derived classes), it could be that the undefined behavior is triggered.

  • Another option could be that the compiler, internally, optimizes away the .type, replacing it with the corresponding (default) value at compile-time (early-binding?).

Another hint:

If I replace the declaration of a Request object in the receiver task with a memory allocation on the heal (using pvPortMalloc), I am able to retrieve the correct type by implementing a virtual getType() function member and override it for each child class. However, I’d prefer not to allocate memory on the heap. I know the maximum size of the variable, so why should I?

7

One BIG issue to watch out for is that FreeRTOS being written in C, and the Queue code copying things by memcpy only works for types that are “Plain Old Data” types. Note that the way you have defined your classes, ReadRequest and WriteRequest each have TWO member of type RequestType with the name Type, one in the base class, and the second in the derived class, but since you queue is built on the type “Request” it will slice the object and only take the base object of type Request. This just won’t work.

1 Like
8

Thanks for your comment, I was expecting the issue being with memcpy. Can you suggest another method to use Queues retaining polymorphism, or is it better for me to change the structure of the code (i.e., with separate queues)?

9

Pass in the queue a pointer to the object, and make sure the object persists. Polymorphic objects just don’t work well in being processed by C code, which FreeRTOS is.

10

I overlooked that your derived classes shadow the base class member.
In your case I think that’s not what you want. If the derived classes just initialize the (same) type member accordingly the code would work.
As Richard pointed out FreeRTOS queues (and similar other mechanisms like message buffers) transfer binary data like ‘plain old data’ structs or classes and have no idea about the hidden this pointer etc.

11

First of all, I would try simply removing the “type” parameters from the derived classes and replacing them with code in the constructor that would simply set the type to the correct value.

Another idea is to use C semantics. In C, polymorphism is handled with unions. If you create a struct that looks like this:

struct foo {
    enum RequestType type;
    union {
        GenericRequest gr;
        ReadRequest rr;
        WriteRequest wr;
    };
};

And then use this struct type in your Queue, the compiler will automatically determine the largest size and use that as the size of each element in the array. To access the data you would do it like this:

err = xQueueReceive( rqueue , &request, pdMAX_DELAY );

if (request.type == READ_REQUEST)
cout << "Read" <<  request.rr.data << endl;
12

Since he described his types to have virtual functions, it just doesn’t work to try to use a union. Data put into a queue need to be C-like structures which are just data. They can have member functions, but not virtual member functions, and the constructors/destructors need to be trivial, since they WON’T be called when moving the data into/out of the queue.

Yes, you need to break it down into something like that union, but it can’t be his original ReadRequest/WriteRequest classes, but a plain old tagged union of plain old data structures, that the polymorphic stuff is built up from at the receiver.

13 
class Request {
protected:
RequestType type = GENERIC_REQUEST;

public virtual RequestType GetRequestType(void)
{


}

};

class ReadRequest : public Request {

};

class WriteRequest : public Request {

};
14

Initialize type in the Ctor

15

Hi @Gianfry7 - I don’t know what your bug is but I do this all the time.
The general idea is base class includes a type identifier as you’ve done.
A struct containing a union of all the derived type sets the required
queue entry size and provides a method to cast the union back to the base class.
Below is an example from production code.
Hope this helps!

/// Display queue entries are derived from abstract base class Display_queue_entry_BaseT.
/// A virtual function implements the UpdateDisplay operation (in display.cpp).
/// Because we're copying objects into and out of fixed-length queue entries,
/// union class DispReq_Union_T below manages queue entry processing and size...
class Display_queue_entry_BaseT {
  public:
    // Because we've not enabled RTTI, specializations use an explicit type
    enum class FakeTypeID_T {
        Unknown,
        FixedMessage,
        MeasurementSet,
        SetBrightness,
        Mastering,
        AdviseStopped,
    };
    const FakeTypeID_T fakeTypeID;
    uint8_t holdSeconds; //> freeze display for this many seconds after displaying payload
    bool Enqueue() const; //> copy this queue_entry to the back of display input queue
    virtual void UpdateDisplay(void) const = 0;
  protected:
    Display_queue_entry_BaseT(FakeTypeID_T _fakeTypeID, uint8_t _hold) :
        fakeTypeID(_fakeTypeID), holdSeconds(_hold) {};
  private:
    Display_queue_entry_BaseT(); // no default ctor implemented
};



/// Fixed-message from predefined enumeration
class DispReq_FixedMessage_T : public Display_queue_entry_BaseT {
  private:
    bool blinkControlMsg; // this is a blink-control message (internal to display process)
    bool blinkOff; // blank display (then delay and turn it back on)
  public:
    virtual void UpdateDisplay(void) const;
    typedef enum {
        Question_Upload, // "Upload measurement history?",
        Question_Erase,  // "Erase measurement history?",
        Wait_Uploading,  // "Uploading, please wait...",
        Wait_Erasing,    // "Erasing, please wait..."
        Wait_Saving,     // "Saving, please wait..."
        FixedTextCount,  // count of valid indexed-text messages above
        Herald,
    } MsgIdx_T;
    MsgIdx_T msgIdx;
    bool blink; //> annoy user by flashing this message on the display?
    // Three labels to be displayed at bottom of OLED (just above the buttons below the OLED)
    typedef enum { ButtonLabels_NO_blank_YES, ButtonLabels_Count,  ButtonLabels_None, } ButtonLabels_T;
    ButtonLabels_T buttonLabels;
    DispReq_FixedMessage_T(MsgIdx_T _msgIdx, uint8_t _hold, bool _blink=false, ButtonLabels_T _buttonLabels=ButtonLabels_None) :
        Display_queue_entry_BaseT(FakeTypeID_T::FixedMessage,_hold),
        blinkControlMsg(false), blinkOff(false),
        msgIdx(_msgIdx), blink(_blink), buttonLabels(_buttonLabels) {};
};

/// Measurement Set display message: up to 3 measurements depending on personality
class DispReq_MeasurementSet_T : public Display_queue_entry_BaseT {
    void dispMeasurementMode() const;
    void dispSiteLocation() const;
    void dispAssetTag() const;
  public:
    virtual void UpdateDisplay(void) const;
    ProductX::C_MeasurementSet ms;
    typedef enum {
        LIVE,
        HOLD,
        MIN,
        Cnt
    } MeasurementMode_T;
    MeasurementMode_T measurementMode;
    struct ProductYIndividual_S {
        bool displayIndividuals;
        int displayNumbers[3];
        ProductYIndividuals_S() : displayIndividuals(false), displayNumbers{0,0,0} {};
        bool operator == (const ProductYIndividuala_S &x) const { return 0==memcmp((const void*)this, (const void*)&x, sizeof(*this)); };
        bool operator != (const ProductYIndividuala_S &x) const { return !(x == *this); };
    } ProductYIndividuala;
    DispReq_MeasurementSet_T(const ProductX::C_MeasurementSet &_ms, MeasurementMode_T _mode, ProductYIndividuala_S _ProductYIndividuala = ProductYIndividuala_S()) :
        Display_queue_entry_BaseT(FakeTypeID_T::MeasurementSet,0),
        ms(_ms), measurementMode(_mode), ProductYIndividuala(_ProductYIndividuala)
        {};
};

/// SetBrightness: Set brightness (without task contention issues)
class DispReq_SetBrightness_T : public Display_queue_entry_BaseT {
  public:
    virtual void UpdateDisplay(void) const;
    uint8_t brightness;
    DispReq_SetBrightness_T(uint8_t _brightness) :
        Display_queue_entry_BaseT(FakeTypeID_T::SetBrightness, 0), brightness(_brightness) {};
};

/// Mastering prompt displays master as set on PC (plus channel label for ProductY)
class DispReq_MasteringPrompt_T : public Display_queue_entry_BaseT {
    int idx; // prompt for which?
  public:
    virtual void UpdateDisplay(void) const;
    DispReq_MasteringPrompt_T(int _idx) :
        Display_queue_entry_BaseT(FakeTypeID_T::Mastering, 0), idx(_idx) {};
};
/// If most recent display is a measurement, note that processing is stopped
class DispReq_AdviseStopped_T : public Display_queue_entry_BaseT {
  public:
    virtual void UpdateDisplay(void) const;
    DispReq_AdviseStopped_T() :
        Display_queue_entry_BaseT(FakeTypeID_T::AdviseStopped, 0) {};
};

/// Union is the max size of all possible display request objects,
/// and provides a convenient class to receive objects from queue.
/// Must contain *all* possible classes above!
struct DispReq_Union_T {
    union uT {
        DispReq_FixedMessage_T a;
        DispReq_MeasurementSet_T b;
        DispReq_SetBrightness_T c;
        DispReq_MasteringPrompt_T d;
        DispReq_AdviseStopped_T e;
        uT() {}; // default ctor required (for DispReq_Union_T's default ctor)
    } u;
    Display_queue_entry_BaseT const & GetBaseDisplayRequest()
        { return (Display_queue_entry_BaseT&)u; };
};