Jump to content
  • Advertisement
Sign in to follow this  
Quat

goto for error cleanup

This topic is 2561 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

Recently I reconsidered exception handling for C++ again. However, after reading the pros and cons, I decided to just use HRESULTS for my rendering engine library. So this led to an issue of how to handle memory cleanup in the case of errors in the middle of a method. Take for example pseudocode like this:



HRESULT MyClass::Foo()

{
HRESULT hr = S_OK;

ResourceA* r1 = 0;
hr = AllocateResource(&r1);
if(FAILED(hr))
return hr;

ResourceA* r2 = 0;
hr = AllocateResource(&r2);
if(FAILED(hr))
{
SafeFree(r1);
return hr;
}

// Use r1 and r2

SafeFree(r1);
SafeFree(r2);

return hr;
}




The above is kind of annoying. If I get an error in the middle of several allocations, I need to clean up all the previous allocations before exiting. I noticed that the Direct3D Effects library uses the following pattern, and it seems like a structured way to use "goto" such that it is not bad form:



HRESULT MyClass::Foo()
{
HRESULT hr = S_OK;

ResourceA* r1 = 0;
hr = AllocateResource(&r1);
if(FAILED(hr))
goto lExit;

ResourceA* r2 = 0;
AllocateResource(&r2);
if(FAILED(hr))
goto lExit;

lExit:
SafeFree(r1);
SafeFree(r2);

return hr;
}




What do you guys think? Without exception handling it seems like this is the cleanest pattern for cleaning up when allocating resources (through COM or other APIs).

Share this post


Link to post
Share on other sites
Advertisement
It seems you encountered a major con for eschewing the built-in C++ way of exception handling. Weird. You'd think the language designers didn't know what they were doing over the past few decades.

The traditional C way of handling this situation is to ignore the errors and leak resources. It's faster, and gives cleaner, more readable code.

Another alternative is to use deeply-nested if block or deeply nested function calls.

Finally, you could do what you've done and try to emulate C++ using gotos.

Share this post


Link to post
Share on other sites
RAII is your friend.

No need to use exceptions if you don't want, but passing on RAII in C++ is... well, you're begging for pain :-)

Share this post


Link to post
Share on other sites
Important point is exceptions aren't actually needed here. As ApochPiQ points out, RAII is sufficient:

[source]
struct ResourceHolder;
{
ResourceHolder(){ r = createResource(); }
~ResourceHolder(){ freeResource( r); }

Resource *r;
}

HRESULT something()
{
ResourceHolder a;

if(error(a->r)) return S_ERROR;

ResourceHolder b;
ResourceHolder c;

if(error(c->r)) return S_ERROR;

return S_OK;
}
[/source]

There can be good reasons to not use exceptions in a C++ project. I can't think of any good reasons not to use destructors. :)

Share this post


Link to post
Share on other sites
Note that for pointer resources, there are probably already smart pointer implementations that you can use without writing your own RAII container. Ex: CComPtr for COM, boost::/std::shared_ptr if you want to specify a custom deleter and want reference counting, std::unique_ptr if you want to specify a custom deleter and don't want reference count, etc.

Share this post


Link to post
Share on other sites

RAII is your friend.

No need to use exceptions if you don't want, but passing on RAII in C++ is... well, you're begging for pain :-)


I once tried to map WinAPI resources to RAII, but gave up.

A proper implementation needs to perform value copy. For majority of resources that can be done, but is quite inefficient. Alternative would be heap allocations and resource tracking, but that again introduces considerable overhead compared to straight C code.

WinAPI error and resource handling is a pain, I remember reading the same discussion some 15 years ago in some printed magazine, which evaluated 3 alternatives, concluding that neither is a win and all bring considerable drawbacks. The boilerplate sequence turns out to be quite reasonable, considering it's familiar.


On higher level, it's the problem dependency injection solves. Resource allocation is decoupled from resource usage. The C version would mean declaring handles once in outermost scope, then use them only from inner scopes without doing any reallocations. It's functionally equivalent to dynamic heap allocation, it just avoids yet another orthogonal resource management and complications associated with it.

RAII is sufficient:[/quote]

It's not. You need to implement copy semantics. Each resource has associated rules for copying, making it a horribly time consuming task. There's also impedance mismatch on copy semantics, WinAPI declares a lot of different sharing models beyond what C++ does.

At minimum, one would need to declare such classes non-copyable, but that has similar effect to const-correctness in how it propagates, forcing entire application to use SSA, which is incompatible with certain API concepts.

For a real-world example, try to convert some of NeHe's tutorials completely. Just the initialization requires definition of dozens of RAII resource wrappers, where some cannot be fully correctly mapped to RAII (module and instance related mostly), leading to basically broken RAII. Many of the others, such as Icons and similar boilerplate need to perform redundant copies during normal usage if used "as intended" or "correctly".

Share this post


Link to post
Share on other sites

...where some cannot be fully correctly mapped to RAII (module and instance related mostly), leading to basically broken RAII.

Can you give a concrete example?

Share this post


Link to post
Share on other sites

Can you give a concrete example?


hInstance gets passed into application, so it cannot be RAII, but one can obtain same type of handle in other ways. At same time, this value needs to be passed to create most resources, so what do you pass? HMODULE, which is not RAII, or something posing as RAII? Or some third unfamiliar type, such as ModuleRef? Or is it even module and should be HWND for some calls?

Even though many API calls use simply HWND, different functions expect different types of handles which also imply different resource policies. I remember something DLL-related was a mind-bender on how many different resources a single handle may represent (don't recall details) along with life-cycle management.

Then there's is general non-determinism, such as GetModuleHandle:
If lpModuleName does not include a path and there is more than one loaded module with the same base name and extension, you cannot predict which module handle will be returned.[/quote]

So even though your code is correct as RAII, the underlying API makes it incorrect:Module m("foo.exe"); // may result in different behaviorAttempting to avoid such ambiguity results in leaky abstraction.

HMODULE also has different behavior, depending on how it was obtained and may or may not be managed:
The GetModuleHandle function returns a handle to a mapped module without incrementing its reference count. However, if this handle is passed to the FreeLibrary function, the reference count of the mapped module will be decremented.[/quote]


It is possible, in theory at least, to map WinAPI to RAII/OO model, but attempting to do so fully will result in monstrosity.


Main conflict comes from various resources being managed internally (or not) based on use. So type, which is preferred classifier (HMODULE) isn't the proper abstraction to build from, instead, the Hungarian Notation part defines the usage. One might have hmInstance (handle to managed instance) and huInstance (handle to unmanaged instance) or similar, mapping to UnmanagedModule um("foo.exe");
ManagedModule ...
Or, alternatively, use GetModuleHandleEx and specify flags, but when trying to do this I soon found that parity between regular and *Ex functions isn't complete. So again there is no consistent mapping. Then I also ran across a case where certain handle/variable/something needs to be passed to certain call in slightly incorrect way, perhaps as abuse to avoid overcomplicated conversions.

WinAPI has so many special cases, especially if trying to cover backward compatibility, that is makes reliable conversion impossible.


But it's been a long time, I just remember that after reading through entirety of some of these API functions I gave up on trying to make a reasonable RAII-based layer. It would be doable, but effort needed vastly exceeded anything justifiable.

Share this post


Link to post
Share on other sites

hInstance gets passed into application, so it cannot be RAII, but one can obtain same type of handle in other ways.

Then have your wrapper accept a custom deleter that can be a null op.

Then there's is general non-determinism, such as GetModuleHandle:
If lpModuleName does not include a path and there is more than one loaded module with the same base name and extension, you cannot predict which module handle will be returned.[/quote]
[/quote]
That's not an issue with RAII; it's an issue with the API. You have the same problem no matter what system you use to acquire and free resources. RAII doesn't magically make things you can't do with an API possible and it's not supposed to either.


HMODULE also has different behavior, depending on how it was obtained and may or may not be managed:
The GetModuleHandle function returns a handle to a mapped module without incrementing its reference count. However, if this handle is passed to the FreeLibrary function, the reference count of the mapped module will be decremented.[/quote]
[/quote]
Again, allow a custom deleter for your RAII class that can be a null op.

Share this post


Link to post
Share on other sites
The the underlying API calls perform inconsistently, then in my view you have two options:

1. Model the inconsistencies in your RAII class type. I.e. if the function you're calling returns a handle that does not need to be released in a specific set of circumstances, then look for those circumstances and set a flag recording whether it needs to be released of later or not.

2. Allow the user to specify the behaviour. Basically just like how some smart pointer types which take as a parameter to their attach method to say whether or not the attach causes an addref. You can add an optional flag to the constructor which allows the user to specify what they expect the behaviour will be for that instance.

I've not yet run into something that I can't wrap acceptably for the Win32 API, but you certainly do have to go to a little extra trouble sometimes. I also don't strictly wrap everything all the time, so it's possible I just didn't even bother in some cases where it would have turned out to be very hard, and then forgot about those cases.

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

GameDev.net is your game development community. Create an account for your GameDev Portfolio and participate in the largest developer community in the games industry.

Sign me up!