How to encapsulate a C API into RAII C++ classes?

Question

Given a C API to a library controlling sessions that owns items, what is the best design to encapsulate the C API into RAII C++ classes?

The C API looks like:

HANDLE OpenSession(STRING sessionID);
void CloseSession(HANDLE hSession);
HANDLE OpenItem(HANDLE hSession, STRING itemID);
void CloseItem(HANDLE hItem);

Plus other functions that are useful for one of these types (Session, or Item) and map directly to C++ member functions of the relevant object. But they are not needed here. My main interest is in the construction and destruction of these objects, using RAII to manage a correct opening and closing of these classes.

My first idea for the design of my classes, is pure and direct RAII. The contained class accepts a container object as the constructor parameter.

class Session {
    HANDLE const m_hSession;
public:
    Session(STRING sessionID): m_hSession(OpenSession(sessionID)) {}
    ~Session() { CloseSession(m_hSession); }
};
class Item {
    HANDLE const m_hItem;
public:
    Item(HANDLE hSession, STRING itemID): m_hItem(OpenItem(hSession, itemID)) {}
    ~Item() { CloseItem(m_hItem); }
};

This design have the disadvantage of allowing a bad behavior: a Session object can be destructed (and CloseSession function called) before all of its Item objects have been destructed. This is annoying, because it shouldn't happened. Even if this erroneous behavior is possible, hence not valid, using the C API, I'd like it to be avoided by design in the C++ API.

That's why I am wondering about using the following design where the Session contains its Items (this shows the actual relationship), and is the sole class able to construct and destroy Items.

class Item {
    HANDLE const m_hItem;
    Item(HANDLE hSession, STRING itemID): m_hItem(OpenItem(hSession, itemID) {}
    ~Item() { CloseItem(m_hItem); }
    friend class Session;
public:
};
class Session {
    HANDLE const m_hSession;
    typedef vector<Item *> VecItem;
    VecItem m_vecItem;
    Session(STRING sessionID): m_hSession(OpenSession(sessionID)) {}
    ~Session() {
        for (size_t n = 0 ; n < m_vecItem.size() ; ++n) delete m_vecItem[n];
        m_vecItem.clear();
        CloseSession(m_hSession);
        }
public:
    Item * OpenItem(STRING itemID) {
        Item *p = new Item(m_hSession, itemID);
        m_vecItem.push_back(p);
        return p;
        }
    void CloseItem(Item * item) {
        VecItem::iterator it = find(m_vecItem.begin(), m_vecItem.end(), item);
        if (it != m_vecItem.end()) {
            Item *p = *it; m_vecItem.erase(it); delete p;
            }
        }
};

It looks to me as the only way to ensure a Session is not closed before its Items are closed: reflecting in the design that the Item objects are members of the Session, and therefore will be destructed before the Session is destroyed.

However, it looks a bit weird to me, as it leaves these functions OpenItem and CloseItem in the interface of the Session class. I was looking for something more in the line of RAII (for me, this means using a constructor for Item), but cannot imagine a way to encapsulate it that would ensure correct destruction order.

Furthermore, using pointers, new and delete is too much old century C++. It should be possible to use a vector of Item (instead of Item*), at the price of defining correctly the move semantics for class Item, but it would be at the price of allowing a default constructor for Item that would create uninitialized second class citizen Item objects.

Any better design ideas?

Answer 1

By adding another layer (and making your RAII a little more explicit) you can get something pretty neat. Default copy constructors and assignment for Sessions and Items do the right thing. The HANDLE for the session will be closed after the HANDLE for all the items is closed. There's no need to keep vectors of children around, the shared pointers track all that for you ... So I think it should do everything you need.

class SessionHandle
{
   explicit SessionHandle( HANDLE in_h ) : h(in_h) {}
   HANDLE h;
   ~SessionHandle() { if(h) CloseSession(h); }
};

class ItemHandle
{
   explicit ItemHandle( HANDLE in_h ) : h(in_h) {}
   HANDLE h;
   ~ItemHandle() { if(h) CloseItem(h); }
};

class Session
{
   explicit SessionHandle( STRING sessionID ) : session_handle( OpenSession(sessionID) )
   {
   }
   shared_ptr<SessionHandle> session_handle;
};

class Item
{
   Item( Session & s, STRING itemID ) : 
     item_handle( OpenItem(s.session_handle.get(), itemID ) ), 
     session_handle( s.session_handle )
   {
   }
   shared_ptr<ItemHandle> item_handle;
   shared_ptr<SessionHandle> session_handle;
};

Answer 2

Use shared_ptr: http://www.boost.org/doc/libs/1_44_0/libs/smart_ptr/sp_techniques.html#handle

Answer 3

This is an interesting problem I think.

First of all, for RAII, you usually want to implement the Copy Constructor and the Assignment Operator in general, here the HANDLE const would prevent them, but do you really want objects that can't be copied ? And better make them exception safe too.

Also, there is the issue of id: do you have to ensure uniqueness or does the framework do it for you ?

EDIT:

The requirements have been precised since my first answer, namely:

• the library implements reference counting already, no need to handle it ourselves

In this case, you have two design alternatives:

• Use the Observer Pattern: the Item is linked back to the Session it was created with, the Session notifies it when it dies (using a session manager this is automated by having the session manager owning the session and having the Item query the manager about its session)
• Use @Michael's scheme in which the Items share the ownership of the Session object, so that the Session cannot be destroyed while at least one Item still exist.

I don't like the second solution much because the lifetime of the Session is much harder to track then: you can't reliably kill it.

On the other hand, as you said, the first solution implies the existence of null objects, which may not be acceptable.

Old Solution:

As for the actual design, I would propose:

class Item
{
public:
  Item(): mHandle() {}

  Item(Session& session, std::string id): mHandle(session.CreateItem(id))
  {
  }

  void swap(Item& rhs)
  {
    using std::swap;
    swap(mHandle, rhs.mHandle);
  }

  void reset()
  {
    mHandle.reset();
  }

  /// Defensive Programming
  void do()
  {
    assert(mHandle.exists() && "do - no item");
    // do
  }

private:
  boost::weak_ptr<HANDLE const> mHandle;
};

And the Session class

class Session
{
public:

private:
  typedef boost::weak_ptr<HANDLE const> weak_ptr;
  typedef boost::shared_ptr<HANDLE const> shared_ptr;
  typedef boost::unordered_map<std::string, shared_ptr> map_type;

  friend class Item;
  struct ItemDeleter
  {
    void operator()(HANDLE const* p) { CloseItem(*p); }
  };

  weak_ptr CreateItem(std::string const& id)
  {
    map_type::iterator it = mItems.find(id);
    if (it != mItems.end()) return it->second;

    shared_ptr p = shared_ptr(new OpenItem(mHandle, id), ItemDeleter());
    std::pair<map_type::iterator, bool> result =
      mItems(std::make_pair(id, p));

    return result.first->second;
  }

  map_type mItems;
  HANDLE const mHandle;
};

This conveys the meaning you asked for:

• The Session object is responsible for managing the lifetime of Items, the actual Item object being no more than a proxy to the handle
• You have a deterministic lifetime of your objects: whenever the Session dies, all the HANDLE to items are effectively closed

Subtle issues: this code is not safe in a multithreading application, but then I have no idea whether we need to fully serialize accesses to OpenItem and CloseItem as I don't know if the underlying library is thread-safe.

Note that in this design, the Session object cannot be copied. Obviously we could create a SessionManager object (typically a singleton, but it's not necessary) and have him manage the Sessions in the very same way :)