Freeing local buffers when throwing exceptions in C++

Question

Suppose I have a following constructor in C++ class:

MyClass::MyClass()
{
    char* buffer = malloc(100);
    if (0 != someD3DXCallThatCanFail(..., buffer, ...))
    {
        free(buffer);
        throw MyException(L"some message");
    }
    char* buffer2 = malloc(200);
    if (0 != anotherD3DCallThatCanFail(..., buffer2, ...))
    {
        free(buffer);
        free(buffer2);
        throw MyException(L"another message");
    }
    .. more code here where buffer and buffer2 are still used

    free(buffer);
    free(buffer2);
}

EDIT: I do hate malloc/free and new/delete, but unfortunately I need to use buffers for loading textures which are then passed over to ID3D10ShaderResourceView, ID3D10Buffer, vertex buffer and the like. All those require pointers to a buffer.

What I am trying to do it to use exceptions instead of returning error codes. Also I wish to create buffers where they are needed and free them right after they are no longer needed.

However, what looks ugly is that in case of errors, no matter if I return error codes or throw exceptions, I still should remember to clean up any buffer that was created up to that point. If I have 10 buffers and 10 possible points of error, I will have to call free() 100 times (in each error case remember to free each buffer).

Now suppose I or worse, my colleague wants to change some logic and, say, adds another buffer somewhere in the middle. Now, he'd have to look thru all the errors that can happen in the rest of the method and add free() for that buffer at every such place. If he's in a hurry, he can easily overlook a few such places, and you got a memory leak.

This also bloats the code immensely.

finally keyword would solve that problem in Java or C#. No matter where in the code the exception happened, I'd still clean up all those buffers in "finally" (btw you wouldn't need that with garbage collection). In C++ from what I understand, I may have to create a member variable for any such buffer, and in the destructor make sure the buffers are cleaned up. Looks also quite ugly to me, since a member variable with name "pBuffer", even a private one, is just garbage, since it's only used in the one method (in this case, constructor) and will always be NULL the rest of the time.

Must be a common problem, however I didn't manage to find an answer using search. Thanks!

You should get out of the habit of using `malloc/free` in C++ unless it's absolutely necessary. Prefer using `new/delete` instead. — Dave Rager, Dec 06 '11 at 20:04
True, but prefer not using `new/delete` either :) Use containers, smart pointers and the like to avoid manually managing memory. They make it much easier to guarantee exception-safety. — Stuart Golodetz, Dec 06 '11 at 20:08
@Dave - and once you've done that, avoid `new` and `delete` using smart pointers for an even simpler life — Steve Townsend, Dec 06 '11 at 20:09

score 8 · Accepted Answer · answered Dec 06 '11 at 20:04

Stop managing memory manually and you won't have these sorts of problems. Use something like std::vector<char>.

Alternatively, you could use something like Boost's shared_array, but that's overkill for what you're trying to do here:

http://www.boost.org/doc/libs/1_41_0/libs/smart_ptr/shared_array.htm

The more general point to be made here is that you should use the RAII idiom - that is, when you acquire resources, you store them in an instance of a class whose destructor frees them again. Then, however an instance of that resource-owning class goes out of scope, the resources are guaranteed to be freed.

See here:

http://en.wikipedia.org/wiki/Resource_Acquisition_Is_Initialization

score 4 · Answer 2 · answered Dec 06 '11 at 20:02

4

The canonical answer is the principle of "Resource Acquisition Is Initialization" (RAII) and smart pointers. You create a class instance on the stack that will free the memory in its destructor, e.g. boost::scoped_ptr.

answered Dec 06 '11 at 20:02

thiton

35,651
4
70
100

1

Thanks, I now I understand the idea of RAII from your and others' answers)) – iseeall Dec 06 '11 at 20:31

score 2 · Answer 3 · answered Dec 06 '11 at 20:04

Instead, use raii:

MyClass::MyClass()
{
    std::vector<char> buffer(100);
    if (0 != someD3DXCallThatCanFail(...))
    {
        throw MyException(L"some message");
    }
    std::vector<char> buffer2c(200);
    if (0 != anotherD3DCallThatCanFail(...))
    {
        throw MyException(L"another message");
    }
    .. more code here 
}

score 1 · Answer 4 · answered Dec 06 '11 at 20:03

1

Use idiomatic C++ approach for this: RAII. This Wikipedia page has C++ sample.

answered Dec 06 '11 at 20:03

Victor Sorokin

11,878
2
35
51

score 1 · Answer 5 · edited May 23 '17 at 12:14

1

Search again, for "C++ smart pointers". You need an exception-safe wrapper on the memory instead of raw malloc, which as you've noted brings a lot of headaches, and incidentally may be better replaced by operator new now you are writing C++ and not C code.

Previous answer here covers this area.

edited May 23 '17 at 12:14

Community

1
1

answered Dec 06 '11 at 20:03

Steve Townsend

53,498
9
91
140

score 1 · Answer 6 · answered Dec 06 '11 at 20:12

The canonical answer for this will be unique_ptr in C++11. Until then, probably scoped_ptr (http://www.boost.org/doc/libs/1_47_0/libs/smart_ptr/scoped_ptr.htm) for singletons and scoped_array (http://www.boost.org/doc/libs/1_47_0/libs/smart_ptr/scoped_array.htm) for arrays, both from Boost. Or you can code the equivalent yourself.

Freeing local buffers when throwing exceptions in C++

6 Answers6