Making a non-object resource RAII-compliant

Question

in my code I use HANDLEs from windows.h. They are used like

HANDLE h;
if (!openHandleToSomething(arg1, arg2, &h)) {
    throw std::exception("openHandleToSomething error");
}
/* Use the handle in other functions which can throw as well */
if (!CloseHandle(h)) {
    throw std::exception("closeHandle error");
}

As you see, you have to insert this CloseHandle to every exception which can happen in the middle of acquiration and release. Therefore, it's likely you forget one (or there is a fancy SEH exception which you didn't know about) and voilà, you have your memory leak.

Recently, I've read about RAII which should remove the headaches from such cases and should call this CloseHandle automatically. I've also seen that there is something like std::auto_ptr<someType> in C++ which solves the problem for resources which were allocated with new.

However, since I don't use new and since HANDLE is just typedefed to be a void *, I wonder how I should use the std::auto_ptr<someType>. Somehow, it should be possible to give it a custom deleter function (if (!CloseHandle(h)) { throw std::exception("closeHandle error"); }). Creating a class would be another method since the destructor gets called any time an instance of it gets out of scope. However, it's just overkill to have a class for every simple thing.

How can I fix these accidential memory leaks?

Note that I would prefer solutions which are in pure C++ with no libraries and big dependencies except if they are really small and used in most of the environments anyways.

Zea

Answer 1

One idea that comes to mind is to use boost::shared_ptr with a custom deleter.

Troposphere

Yes, that’s one of the things that shared_ptr has been designed to to, and do well. This is definitely the best solution (although I usually avoid such absolutes). – Brume

@dalle: Ah, thanks for adding the link. That's a better example than mine. – Troposphere

@Fred: Cheers! Using that technique prevents the extra new. – Athens

you should never throw from a destructor – Bucktooth

@fnieto: You're right, but this example won't do that. What it will do is ignore any error return from CloseHandle(). It would be possible to create a custom deleter that would handle the error without throwing. – Troposphere

Smart pointers is the wrong solution for this problem. What you need is a simple wrapper. – Marquetry

One of the conditions is "I would prefer solutions which are in pure C++ with no libraries". Why to use shared_ptr for so little? – Bucktooth

@fnieto: "...except if they are really small and used in most of the environments anyways." I think shared_ptr fits that description. – Troposphere

shared_ptr is practically pure C++ (get any new compiler) – Marigolda

@Martin: Actually, I think the "simple wrapper" examples you and fnieto have offered are more complicated, more error-prone, and less functional (non-copyable) than the shared_ptr solution, even if a custom deleter is required. – Troposphere

hmm. using visual studio 2008 professional and the microsoft c++ compiler. does it know this shared_ptr or do I have to install boost separately? – Zea

Not sure about VS2008, but a lot of compilers have shared_ptr available as part of tr1. – Troposphere

@Fred: the problem with shared_ptr is that it lacks the close() function which Martin's solution provides. With a bit of effort, he could provide reference-counting semantics if required, but your solution provides no way for users to detect errors closing the handle. Knowing about such errors might be critical for the app, if for example it is trying to implement transactions, and an exception on close indicates an error. If the handle can be flushed in such a way that close is guaranteed to succeed, then your solution is fine, but Martin's is more general. – Marolda

@onebyone: The way to handle close errors with this technique is to add a deleter functor to call CloseHandle() and handle any errors. – Troposphere

@etan: I think shared_ptr should be there. Maybe someone more familiar with VS2008 can help you find it. Sorry. – Troposphere

have found it out. need SP1 to update the <memory> file. without sp1 you won't have it. – Zea

@Fred: sure, but then the deleter functor has close over different state for each caller, because they want different side effects on failure. Each caller has to write their own deleter, rather than just wrapping the handle in a shiny new "C++ interface" that everyone uses. Inversion of control can go too far. Unless you need the reference counting, I think this will rapidly get as lengthy and error-prone as writing a correct wrapper once. – Marolda

@Fred: We obviously disagree on what is complexity. But Shared pointer is obviously the wrong smart pointer. At best this should be a scopped pointer. There is no requirement to pass or share ownership through any other part of the code. It is created and destroyed in a single place that just needs exceptions safety. – Marquetry

@Fred: Also using a shared pointer like this means you cant implement the deleter correctly (as it cant throw). This is a requirement in the question above and in the genral case for this type of problem. If you think it is solvable then you should provide some code to prove your point. – Marquetry

Ok, looking at this again I realize that the issue is that the deleter cannot propagate an exception to be handled elsewhere. That is indeed a limitation, and I don't know of a way it can be overcome. But it can be useful in cases where the error can be handled by a deleter functor or where the error can be safely ignored. @Martin, scoped pointer can't be used for this technique as it does not allow specifying a deleter. I believe shared_ptr is the only boost/tr1 smart pointer that can be used for this technique. – Troposphere

The problem with shared_ptr<> isn't the deletion (custom deleter works) but the copy ctor should relly call DuplicateHandle() in this case. – Bannerol

Answer 2

You can implement your own simple RAII idiom.

class auto_handle {
public:
    auto_handle() : handle_() {}
    ~auto_handle() {
        if (!CloseHandle(handle_)) {
            // Don't throw here (1), manage the error in other way.
        }
    }
    HANDLE& handle() { return handle_; }
private:
    auto_handle(const auto_handle&);
    auto_handle& operator=(const auto_handle&);
    HANDLE handle_;
};

(1) You should never throw from a destructor.

auto_handle h;
if (!openHandleToSomething(arg1, arg2, &h.handle())) {
    throw exception("openHandleToSomething error"); // Now it is safe
}

Bucktooth

I think you have issues here if you copy the handle. Each copy will try to close the same handle. – Troposphere

what does handle_() and handle_(h) ?. handle is just a void*, how to call a function on it? – Zea

seems as it needs at least the default constructor since it doesn't compile otherwise. however, it doesn't seem to work. I just get a compiler error since it cannot convert from 'auto_handle *' to 'PHANDLE' – Zea

Why is the copy ctor private/unimplemented? DuplicateHandle exists for a reason. Note: It's OK for a copy ctor to fail by exception. – Bannerol

This class is intended to keep the ownership of the handle and close it when goes out of ambit. The private copy constructor is to avoid it to be copied or duplicate. That thing is out of the question scope and would imply reference counting (Or DuplicateHandle, what I didn't know). – Bucktooth

Answer 3

1) Don't use auto_ptr<>. Seriously. You don't want those headaches-- it's far too easy to slip up b/c it doesn't have familiar copy semantics.

2) Wrap HANDLE with a simple object that provides an accessor that gives you the underlying handle. You'll need this to pass the HANDLE in to API calls. (I'd consider an accessor preferable to an implicit conversion.)

3) I've never actually bothered wrapping HANDLE, so I don't know if there are any surprising gotcha's. If there are, I can't point them out. I wouldn't expect any-- it's an opaque value. But then, who expects a surprising gotcha'? They're surprises, after all.

4) (Of course) implement the appropriate dtor.

Primateship

Your point 1 is incorrect. auto_ptr has pointer semantics, and it has its uses (improved by C++0x’ rvalue references). However, it simply cannot be used as a solution to this problem since it lacks the required features. – Brume

@fnieto: Who said anything about an ancestor? – Primateship

Answer 4

std::auto_ptr is not suitable for this situation. It has its uses, but this isn't one of them. To correct (sort of) a point raised by Greg D, the problem with auto_ptr isn't so much its lack of pointer semantics, as its rather odd ownership semantics -- when you assign one, you don't get a copy of the pointer, but instead a transfer of the pointer (i.e the assignee becomes the new sole owner of the resource, and the assigner no longer has anything).

You do want to wrap the handle in a class though. I've done this a number of times, and it works quite well. I haven't run into anything particularly surprising when doing it, though that doesn't necessarily mean a lot -- handles are used for a lot of things in Windows, and some of them might easily have some oddities.

Subcartilaginous

Answer 5

You just want a simple wrapper that gives you the handle when you pass it into a function:

#include <stdexcept>
class HWrapper
{
    HANDLE h;
    bool   closed;

    public:
        HWrapper(A1 arg1,A2 arg2)
            :closed(false)
        {
            if (!openHandleToSomething(arg1, arg2, &h))
            {    throw std::runtime_error("openHandleToSomething error");
            }
        }
        ~HWrapper()
        {
            try
            {
                if (!closed)
                {   close();
                }
            }
            catch(...) {/*Exceptions should never leave a destructor */ }
            // Though you may want to log somthing.
        }
        void close()
        {
            closed = true;
            // Close can throw an exception.
            if (!CloseHandle(h))
            {    throw std::runtime_error("closeHandle error");
            }
        }

        /*
         * This allows you to just pass it to a function that takes an HANDLE
         * See the function:   functionThatUsesHandleButMayThrow();
         */
        operator HANDLE()
        {
            return h;
        }
    private:
    /*
     * For your use case there is not need to copy.
     * So explicitly disallow copying.
     *
     * Just pass the HWrapper object to any function that requires a handle.
     * The built in cast operator will convert it back to a Handle to be used
     * within these functions. While this object just retains ownership and
     * responcability for deleting the object when you are finished.
     *
     * This allows simple backwards compatibility with existing code.
     */ 
    HWrapper(HWrapper const& copy);            // Don't implement
    HWrapper& operator=(HWrapper const& copy); // Don't implement


};

void functionThatUsesHandleButMayThrow(HANDLE h)
{
}



int main()
{

    try
    {
        HWrapper   w(A1,A2);

        functionThatUsesHandleButMayThrow(w);
        /*
         * If you do not care about close throwing an excepion.
         * Then jsut let it fall out of scope. The destructor
         * will try and clean up. But if it fails it will drop the
         * exception.
         *
         * This is required because if another exception is propogating
         * throwing an exception terminates the application.
         */
    }
    catch(std::exception const& e)
    {
        std::cout << "Exception: " << e.what() << "\n";
    }


    try
    {

        HWrapper   w2(A1,A2);

        functionThatUsesHandleButMayThrow(w2);
        /*
         * If you do care abou the exception
         * The call close() manually. The exception will be thrown.
         *
         * But if an exception is already been thrown in
         * functionThatUsesHandleButMayThrow() then we will try and close it
         * in the destructor and not throw another exception.
         */
        w2.close();
    }
    catch(std::exception const& e)
    {
        std::cout << "Exception: " << e.what() << "\n";
    }
}

Marquetry

Answer 6

HANDLE h;
if (!openHandleToSomething(arg1, arg2, &h)) {
    throw std::exception("openHandleToSomething error");
}

Here it is :

auto d = [](decltype(h)* a) { if(a) ::CloseHandle(*a); };
std::unique_ptr<decltype(h), decltype(d)> buf(&h, d);

Catima