No RAII to copy - xfgr.com

Email Password Remember me?

A hidden danger of wrapping using RAII rears its head when we start copying the object around. Let's look at why and what we can do.

You may recall our RAII MIDI class from before:

class CMidiOut
{
public:
    CMidiOut()  :
        m_hMidiOutHandle(nullptr)
    {
        midiOutOpen(
            &m_hMidiOutHandle, 	// Handle
            MIDI_MAPPER,    	// Default device
            NULL,      		// No callback
            NULL,      		// No callback parameters
            CALLBACK_NULL);   	// Flags
    }
    ~CMidiOut()
    {
        if(m_hMidiOutHandle != nullptr)
        {
            midiOutClose(m_hMidiOutHandle);
            m_hMidiOutHandle = nullptr;
        }
    }
private:
    HMIDIOUT m_hMidiOutHandle;
};

Clients of the class no longer have to worry about the raw API calls, nor do they have to remember to close the handle and can write exception happy code. A danger lies in the compiler generated copy constructor though, and also the assignment operator. These default implementations will take a copy of our handle, which is fine, but as soon as one of the copies (or the original) is destroyed, there will be a call to midiOutClose. This is also okay, until one of the copies tries to use its handle. The handle has been closed. Our resource has been unacquired. Midi plays no more.

We would write defensive code to protect against this situation. We have two alternatives:

Prevent copying of the RAII class.
Use reference counting on duplicating the handle.

Of the two options, number 2 is by far the most interesting, but sadly we won't be using it here. DuplicateHandle does not work on HMIDIOUT and a reference count solution would work but is far too complicated for what we need. Instead, we are going to simply turn the class into a pure wrapper for HMIDIOUT and prevent any copies of it. This is done by making the copy constructor and assignment operator private:

class CMidiOutHandle
{
public:
    CMidiOutHandle();
    ~CMidiOutHandle();

private:
    CMidiOutHandle(const CMidiOutHandle &);
    CMidiOutHandle& operator=(const CMidiOutHandle&);

private:
    HMIDIOUT m_hMidiOutHandle;
};

Note also the name change. We are now making it clear that this class can be used in place of a HMIDIOUT (and it will, once we get to the accessor members).

Scott Meyer's in his excellent Effective C++ describes a reusable base class for copy prevention:

class Uncopyable
{
protected:
    Uncopyable();
    ~Uncopyable();

private:
    Uncopyable(const Uncopyable &);
    Uncopyable& operator=(const Uncopyable&);
};

There is also a noncopyable class in Boost, which does pretty much the same thing. You can then simply inherit this from your noncopyable classes:

class CMidiOutHandle : private Uncopyable

I do like the self documentation here, but I don't actually like to use the base class method. If you try and copy the object using the first method, it will error and helpfully point you to the line which is doing the copying. If inheriting from the non copyable base class, the compiler will say the error is caused by your class. Not a big deal, but enough for me to explicitly disallow the copy constructor and assignment operator.

You must Sign In to use this message board.


Per page

"...and a reference count solution would work but is far too complicated for what we need..."
peterchen

1:08 21 Jun '10

If a dependency on boost:.shared_ptr is acceptable, it's not so bad:

boost 2: shared_ptr wraps resource handles[^]

struct CMidiOutDeleter
{
   void operator()(HMIDIOUT handle) {  midiOutClose(handle); }
}

typedef CHandleRefT CMidiOutRef;

void midiOutClose(CMidiOutRef); // not implemented, prevent explicit close

The idea of using suhc a HandleRef is to provide a "comfy" open method:

CMidiOutRef MidiOutOpen(...) 
{
   HMIDIOUT handle;
   midiOutOpen(&handle, ...);
   // e.g. throw on error
   return CMidiOutRef(handle);
}

and use that handle in native API's that expect an HMIDIOUT:

CMidiOutRef mo = MidiOutOpen(...);
midiOutShortMsg(mo, ...);

Please elt me know what you think.

Agh! Reality! My Archnemesis![^]

FoldWithUs! sighist µLaunch - program launcher for server core and hyper-v server.

Re: "...and a reference count solution would work but is far too complicated for what we need..."
WebBiscuit

9:21 21 Jun '10

I actually already read your article when I was toying between the ideas of reference counting and non-copyable! I do like your ideas and I resisted created non-copyable classes for a long time as not being able to copy an object felt unnatural to me.
But then I found my abstractions were wrong and I had come to incorrect conclusions. I will be using a shared_ptr but not at the HANDLE level. That name says it all, a shared pointer is for objects that should not be copied, but shared. You can apply this to the raw handle, as you are doing, but I find it easier to take it to a level higher where the shared_ptr wraps the uncopyable managed handle.

shared_ptr<CMidiOutHandle>  // Fine - now the owner of this object can be freely copied

A custom deleter is now no longer needed. RAII is doing what it does best.

Last Updated 20 Jun 2010 Advertise Privacy Terms of Use