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?

Asel

If we agree that the standard RAII wrapping is okay, then there are 2 questions: what is most elegant (RAII) way of opening/closing Items in Session and what is an alternative for the vector of pointers, right? – Disembogue

Ideally, I'd like to have both the elegant RAII way of opening/closing items, and the items belonging to the sessions so that they are closed before the session is. (vector of pointers issue is not the question) – Asel

To me, the 2 examples that you show serve a different purpose whether you want to work with temporary Items (only in the scope of say a function) or if you want the items to be longer living in which case RAII can't help you with clearing up the items as long as the Session is alive. – Disembogue

@Didier: I would like to know if there is a risk of collision of ids. If I pass twice the same id to OpenItem do I get the same HANDLE ? If it is so, then we need to control the call to CloseItem with reference counting to make it properly, which imposes a Factory of sort for both Items and Sessions I guess. – Kev

@Matthieu: If you pass the same id twice to OpenItem, you get the same handle. This is not something I need to avoid as the library has ref counting embedded within. This means that if opened twice, an Item will only be actually closed after the second call to CloseItem. This is a feature I can live with, and I even consider this a liberty for the library user I don't want to remove. – Asel

@Didier: how that's neat, makes for a much easier life for wrapping since you therefore don't really need reference counting! I'll edit my answer then. – Kev

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 Session( 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;
};

Beatty

If OpenItem and OpenSession can return NULL, you might want to either change the constructors to throw in that case, or use the null-object pattern. – Beatty

I don't like that a Session can leak if one of its Item remains alive somewhere. I much prefer deterministic lifetime whenever possible, makes for easier debugging. – Kev

@Matthieu M. In both your example and mine you need to be careful about lifetimes of your objects. In mine if you're not careful you get a leak, in yours you get an error. Its a design decision the user needs to be aware of in both cases. (Having said that I like your design a lot, +1 ) – Beatty

I agree there is a conscious decision to be made, and I don't reject your idea out of hand (for one thing, it is much simpler). It is just that having debugged both cases, I have found that an error was much easier to analyze that a leak, since the very cause of a leak is an unforeseen access / copy. That is why I tend to shy from shared_ptr as much I can, indeed I only used them here because weak_ptr offer a safe and easy way to actually know whether or not the object is still live. – Kev

Answer 2

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 :)

Kev

For RAII, there's no absolute need for Copy Constructor and Assignement Operator. Providing move semantics as Move Constructor and Move Operator is enough. Anyway, this does not avoid the NULL class Item existence. – Asel

@Didier: no it doesn't, but I didn't see this requirement in your question. It does however correctly "nullify" the Item whenever the session dies. If you want the Item to remain valid, then please consider @Michael's answer. You'll have a hard time controlling the lifetime of Session, but you won't invalidate your Items any longer. – Kev

Answer 3

To expand on STLSoft's comment, use STLSoft's scoped_handle smart pointer, as in:

HANDLE hSession = OpenSession("session-X");
if(!hSession) {
 // Handle failure to open session
}
else {
  stlsoft::scoped_handle<HANDLE> session_release(hSession, CloseSession);

  HANDLE hItem = OpenItem(hSession, "item-Y");
  if(!hItem) {
     // Handle failure to open item
  }
  else {
      stlsoft::scoped_handle<HANDLE> item_release(hItem, CloseItem);

    // Use item
  }
}

If the "null" handle value is not 0, then do something like:

if(hSession != -1) {
 // Handle failure to open session
}
else {
  stlsoft::scoped_handle<HANDLE> session_release(hSession, CloseSession, -1);

HTH

Lyricism

Answer 4

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

Jorry