Design Patterns: Sharing Polymorphic Objects with DDS

In this scenario, a node publishes a single topic (for example 'TEMPERATURE') containing information about the node itself (its ID) and all the monitored sensors (up to 5 temperatures).

Internally (in your application) perhaps you want to share only part of the information associated with each object. So in the example above:

• You define a basic object ("Sensor") containing the 'shareable' or 'public' information about a sensor.

• Internally you extend this basic "Sensor" into a more specialized class ("MySensor") that contains some C++ specific methods, pointers to other internal objects, or simply private properties (that are not going to be shared with other nodes).

• Similarly, for each temperature sensor managed by the node, you may want to follow a similar approach, where only a portion of the data associated with a real temperature sensor is published over DDS (a "State" object). 

 The advantages of sharing the internal data model with the middleware is that sharing your state with other applications is just a matter of calling:

myWriter->write(mySensor);

(where 'mySensor'' is an instance of the class 'MySensor').

The middleware will see only the 'Sensor' part and will automatically take care of serializing its content and sending it to any reader interested in mySensor.

// Sensor.idl
// ---------------------------------
@nested
struct State {
  double temp;
  boolean fault;
};

struct Sensor {
  @key
  string id;
  sequence<State, 5> state;
};

$ rtiddsgen -language C++ -replace -constructor-virtualDestructor Sensor.idl

class Sensor {
  public:
    DDS_Char * id ;
    StateSeq  state ;
    Sensor();
    Sensor(const Sensor& that);
    virtual ~Sensor();
    Sensor& operator=(const Sensor& that);
};

class MySensor: public Sensor {
  public:
    MySensor():
          Sensor() { }
    virtual ~MySensor();

    // Define here all the properties that are not going to be
    // shared with other applications
};

Solution #1: Manually manage the sequence by loaning a buffer

The implementation of a sequence in RTI Connext DDS permits you to manage the memory associated with the contained elements. You can loan a contiguous buffer or a discontiguous buffer:

• Contiguous Buffer: The middleware expects an array of the contained object of the specified length:
  Unfortunately this approach won't work, because if we create and loan a static array of contiguous InternalState objects, the middleware will still try to access the memory as "State" objects. Accessing any element beyond the first one will cause random memory corruptions.
  
  

• Discontiguous Buffer: When you loan a discontiguous buffer to the middleware, you are only providing an array of pointers to the objects you want to store in the sequence (then you will be responsible for creating and destroying the individual objects):
  

Perfect! That is what we want. In the constructor of the MySensor class, we can pass an array of pointers to the sequence:

class MySensor {
  private:
    State * internalState[5];

  public:
    MySensor::MySensor(): Sensor() {
      memset(internalState, 0, sizeof(internalState));
      state.maximum(0);      // We will manage the memory
      state.loan_discontiguous_buffer(internalState, 0, 5);
}

  ...
  state.length(1);
  internalState[0] = new InternalState();
  ...

  ...  
  state.length(0);
  delete &state[0];   // We can access the pointer through the seq.
  internalState[0] = NULL; // Good practice, but not required
  ...

Solution #2: Use @external in your IDL and have the middleware manage the memory 

Let's go back to the IDL and change it to instruct the middleware that now the Sensor has a sequence of @external (or pointers) to State objects (instead of having a sequence of State objects): 

// Sensor.idl
// ----------
@nested
struct State {
  double temp;
  boolean fault;
};

@external
typedef State StatePtr;
struct Sensor {
  @key
  string id;
  sequence<StatePtr, 5> state;
};

With this change, the internal sequence stores pointers to the State objects, not the State object itself. Now we can store a pointer to an InternalState object in the sequence, instead of a State object itself. 

Accessing its element is simple. For example, the 1st element can be cast to an InternalState pointer:

  ...
  InternalState *is = static_cast(state[0])
  ...

This approach works well, but you have to be very careful regarding memory management.

To better understand what is going on, suppose you want to add one element to the sequence. An initial approach would be to do something like this:

   ...   
  state.length(1);  // Sequence is resized to have one pointer
  state[0] = new InternalState();
  ...

In this case, your application will leak memory.

When you call:

  state.length(1);

the middleware will always create an instance of a State object and assign it to its position 0. Essentially, internally it is doing something like this:

  ...
  buf[length] = new State();
  ++length;
  ...

To avoid memory leaks, you must remember to always delete this object before assigning your own:

   ...
  state.length(1);  // Sequence is resized to have 1 pointer
  delete state[0];  // Delete the object allocated by DDS
  state[0] = new InternalState(); // Now I can assign my own ptr
  ...

 Since the middleware is managing the memory of the sequence, that includes the pointed objects, too:

• When you expand the sequence, DDS will create an instance of the State object for every new element.

• When you reduce (or delete the entire sequence), DDS will call 'delete' to every non-NULL pointer of the sequence.

 So when you want to remove an element from the sequence, you must remember to set the pointer to NULL before changing the length of the sequence to avoid double-free memory corruption:

   .....  
  delete state[0];  // Destroy my InternalState object
  state[0] = NULL;
  state.length(0);  // Sequence is resized, no double free
  ...

This approach is quite unnatural and extremely error prone (you can easily forget those extra steps every time you manipulate the sequence and end up with memory corruption or memory leaks).

Solution #3: Use @external in your IDL, but manage the memory of the sequence through a contiguous buffer

This solution is similar to the previous approach, but instead of having DDS manage the memory of the sequence like in the first solution, we tell the middleware that we want to manage it. Differently than Solution #1, we manage it through loaning a contiguous buffer (of pointers now) to the sequence (because now our contained values are pointers, not objects): 

class MySensor {
  private:
    State * internalState[5];

  public:
    MySensor::MySensor():
        Sensor() {
      memset(internalState, 0, sizeof(internalState));
      state.maximum(0);    // We will manage the memory
      state.loan_contiguous_buffer(internalState, 0, 5);
}

(Similar to Solution #1, initializing the internalState to all NULL pointers is not required, but it's a good practice).

 Now we can follow exactly the same pattern as Solution #1, without incurring the issue related to dealing with discontiguous buffers.

Adding an element to the sequence does not require you to delete a useless object:

  ...
  state.length(1);
  state[0] = new InternalState();
  ...

 And deleting an element is finally intuitive and does not require you to set the pointer to NULL:

  ...
  delete state[0];
  state.length(0);
  ...

Additional Information

Be careful when you delete objects from the middle of a sequence.

The @external annotation gives you a way to refer to external memory that has been previously allocated, but it does not provide optional semantics. This means that all the elements in the sequence must point to an object and cannot be NULL.

For example, suppose you initialize the sequence with 3 InternalState objects (using solution #3): 

  ...
  state.length(3);
  state[0] = new InternalState();
  state[1] = new InternalState();
  state[2] = new InternalState();
  ...

Then suppose you need to remove the first element (the one in position #0). You cannot just delete that object and set state[0] to NULL. If you do that, you will get an assert error when you try to write the sample containing the sequence.

The correct approach is always to re-compact the sequence:

  ...
  delete state[0];
  state[0] = state[1];
  state[1] = state[2];
  state[2] = NULL;    // Not needed, just good practice
  state.length(2);
  ...

Or, even better, use a generic function like this:

bool removeState(State *s) {
    // Find the position
    int pos = -1;
    for (int i = 0; i < state.length(); ++i) {
        if (state[i] == inst) {
            pos = i;
            break;
        }
    }
    if (pos == -1) {
        // Not found
        return false;
    }
    delete m_instance[pos];
    if (pos < state.length()-1) {
        // Remove 'holes' in the sequence
        for (; pos < state.length()-1; ++pos) {
            state[pos] = state[pos+1];
        }
    }
    m_instance[pos] = NULL; // Not needed, just good practice
    m_instance.length(pos);
    return true;
}