Cleaning up after an exception

[John E]

Suppose my function needs to re-throw an exception - but it needs to clean up before doing so. The obvious solution is somehting like this....

HRESULT MyFunc()
{
	// Declare lots of pointers
	char*	p1 = NULL;
	char*	p2 = NULL;
	char*	p3 = NULL;
	char*	p4 = NULL;

	try
	{
		// Whatever...
	}
	catch ( MyException  )
	{
		// Clean up after caught exception
		if (NULL != p1)
			delete [] p1;

		if (NULL != p2)
			delete [] p2;
		
		if (NULL != p3)
			delete [] p3;
		
		if (NULL != p4)
			delete [] p4;

		// Re-throw the exception
		throw;
	}
	
	// Clean up after normal execution
	if (NULL != p1)
		delete [] p1;

	if (NULL != p2)
		delete [] p2;
	
	if (NULL != p3)
		delete [] p3;
	
	if (NULL != p4)
		delete [] p4;

	// and return normally
	return ERROR_SUCCESS;
}

However, the above strategy requires 2 lots of cleanup code (something I'm always loathe to do because I've found it to be a common cause of programming errors).

Is there a better strategy? - i.e. one that requires only 1 block of cleanup code, regardless of whether an exception occured or not?

[raghupathys]

If you are a C programmer, you can use setjmp / longjmp.

Win32 SEH has the __try / __except / __finally semantics, which would solve your problem. Only problem being, they are meant for C again and are not c++ friendly.

You could also try switching to C# where they have the try / catch / finally semantics.

Or if you need a purely c++ approach, consider using goto.

P.S. Please do not flame me for suggesting goto. It is a feature of the language, and if you use it consistently and intelligently, there is no reason for the code to become unreadable / unmaintainable.

[raghupathys]

In your case you could also move your cleanup code into a separate function. Then you would only have multiple function calls - not duplicate code.

[John E]

Or if you need a purely c++ approach, consider using goto.

P.S. Please do not flame me for suggesting goto. It is a feature of the language, and if you use it consistently and intelligently, there is no reason for the code to become unreadable / unmaintainable.

Ha ha.... I got enough flagellation myself once for starting a debate about 'goto'

[raghupathys]

Oh! So that was you???

That was a really long running series of comments! And pretty interesting reading too!

[Hobson]

Suppose my function needs to re-throw an exception - but it needs to clean up before doing so. The obvious solution is somehting like this....

HRESULT MyFunc()
{
	// Declare lots of pointers
	char*	p1 = NULL;
	char*	p2 = NULL;
	char*	p3 = NULL;
	char*	p4 = NULL;
 
	try
	{
		// Whatever...
	}
	catch ( MyException )
	{
		// Clean up after caught exception
		if (NULL != p1)
			delete [] p1;
 
		if (NULL != p2)
			delete [] p2;
 
		if (NULL != p3)
			delete [] p3;
 
		if (NULL != p4)
			delete [] p4;
 
		// Re-throw the exception
		throw;
	}
 
	// Clean up after normal execution
	if (NULL != p1)
		delete [] p1;
 
	if (NULL != p2)
		delete [] p2;
 
	if (NULL != p3)
		delete [] p3;
 
	if (NULL != p4)
		delete [] p4;
 
	// and return normally
	return ERROR_SUCCESS;
}

However, the above strategy requires 2 lots of cleanup code (something I'm always loathe to do because I've found it to be a common cause of programming errors).

Is there a better strategy? - i.e. one that requires only 1 block of cleanup code, regardless of whether an exception occured or not?

Solution for releasing resources after exception is thrown is to use RAII technique. Simply change your raw pointers to vector, string or some boost managed pointer (shared_array would be good). It deallocates resources atomatically during stack unwinding.

regards
Hob

ADDED: one more thing, when deleting pointer, there is no need to ceck if it is null or not. Calling delete for NULL is guaranted to be safe. Thus, it is good practice to set deleted pointers to NULL. In your above code change

if(ptr!=NULL)
delete []ptr;

to

delete []ptr;
ptr = NULL;

[googler]

Is there a better strategy?

Not really. All the possibilities have been mentioned in the thread, but none of them are "better". IMHO, this is one of the biggest design flaws in C++. Ideally, the language should have a construct such as this...

{
     // ....
      char* p = new char[100];
      AddFinalizer {
            delete[] p;
     }
      // rest of function
}

with the semantics that the finalizer should be called regardless of how the function exits. Unfortunately, it doesn't.

You can use __try/__finally around the whole function body as long as you don't use C++ try/catch and you don't have local variables with destructors in the same function. This is Windows-specific, but since the forum is about Visual C++, I guess that's ok Even the __try/__finally construct is pretty awkward because it forces you to indent the whole function body.

[John E]

Even the __try/__finally construct is pretty awkward because it forces you to indent the whole function body.

In fact, I did a bit of digging (on MSDN) and __try/__finally etc all seems a bit weird to me. For example, the __except block appears (textually) after the __finally block but gets called before it. Apart from which, I still couldn't figrue out how it would allow me to re-throw the original exception, which was my main requirement. RAII and Smart Pointers seem promising though....

[googler]

RAII and Smart Pointers seem promising though....

That's probably the best way to go, since using destructors of local variables is the only official way C++ provides for adding function finalizers. You get code that isn't platform dependent.
The draw-back to this method is that it's cumbersome because you have to create a finalizing class for each type of finalization you want to do. The finalizing class has to package the data that needs to be finalized, and do the finalization code in the destructor. However, for your specific need - deletion of char* - existing finalization wrappers abound, so you don't have to write one yourself.

In fact, I did a bit of digging (on MSDN) and __try/__finally etc all seems a bit weird to me. For example, the __except block appears (textually) after the __finally block but gets called before it.

Don't let their example confuse you. __try/__finally and __try/__except are two different statement constructs. You can nest them any way you want, but you can't write both an __except handler and a __finally handler for the same __try. They always refer to different __trys (which may be nested).

Apart from which, I still couldn't figrue out how it would allow me to re-throw the original exception, which was my main requirement.

The beauty of it is that you don't need to re-throw the exception. The __finally block only gets called to unwind the stack. It has nothing to do with the exception. The dispatching of the exception will automatically continue after your __finally block. If you exit from the __try with normal control flow, it will execute the __finally and continue with normal control flow. If you exit from the __try with an exception, it will execute the __finally and continue throwing the exception!

Here is how to code your example with __try/__finally

HRESULT MyFunc()
{
	// Declare lots of pointers
	char*	p1 = NULL;
	char*	p2 = NULL;
	char*	p3 = NULL;
	char*	p4 = NULL;

	__try
	{
		// Whatever...
	}
	__finally
	{
		// Clean up
		delete [] p1;
		delete [] p2;
		delete [] p3;
		delete [] p4;
	}
	return ERROR_SUCCESS;
}

[John E]

Don't let their example confuse you. __try/__finally and __try/__except are two different statement constructs. You can nest them any way you want, but you can't write both an __except handler and a __finally handler for the same __try

Ah - thanks for explaining that because you're right - it wasn't obvious from their example (well, not to me anyway..! )