SafeRelease not that Safe?

Started by
3 comments, last by cozzie 10 years, 11 months ago

Hi,

I have fairly simple question.

In a lot of cases people use a template/ function or macro to 'Safely release' COM objects.

For example:


#define SafeRelease(x)		if(x){x->Release(); x=NULL;}

But what if we have this situation:


LPD3DXEFFECT			mEffect = NULL;

D3DXCreateEffectFromFileA(pD3ddev, mFilename.c_str(), NULL, NULL, 0, pEffectPool, &mEffect, &errorBuffer));

And the effect is not created for some reason, i.e. filename doesn't exist.

Then in the destructor of the class 'mEffect' is part of I call:

SafeRelease(mEffect);

But this causes a crash and when I don't release the mEffect there's no memory leak. But I just want to keep clean/ nicely released objects.

How would I do that?

Crealysm game & engine development: http://www.crealysm.com

Looking for a passionate, disciplined and structured producer? PM me

Advertisement

You should check the return value from the effect creation method. If it isn't created, then you have an error situation, and have to act accordingly.

Is the value of mEffect non-null even when the call fails? That shouldn't be the case - it should either be null or not return a value in mEffect... This is, of course, assuming that the effect is a COM object (which to be honest, I don't know if it is...).

D3DXCreateEffectFromFileA does not set mEffect if the function fails. Are you sure that you sett mEffect to NULL in your constructor?

Maybe you are having two mEffect, one local before your call to D3DXCreateEffectFromFileA, as you write above, and one as a member in your class that never gets initialized to NULL in the constructor?

If your CreateEffectFromFile call fails then you're already in undefined territory - it could have failed at any point during the creation process and your mEffect variable may or may not have been assigned a value by the time at which it failed. The D3D documentation doesn't specify what value the "__out LPD3DXEFFECT *ppEffect" parameter has on a failure; it just specifies a return value that indicates a failure, so you shouldn't write any code that depends on it having a value - it's perfectly possible to come out with a failure result that has that parameter pointing at junk memory.

So I'll second what's been said above; don't check mEffect, instead do it the official way and check the HRESULT you get back, if that indicates a failure then you can work on the basis that the object is invalid and that you don't need to bother with releasing it.

Direct3D has need of instancing, but we do not. We have plenty of glVertexAttrib calls.

Thanks. I solved it.

The cause was that I didn't set mEffect to NULL in the constructor (stupid).

I'm also checking the result of the create function and if the fails set mEffect to NULL just in case.

Crealysm game & engine development: http://www.crealysm.com

Looking for a passionate, disciplined and structured producer? PM me

This topic is closed to new replies.

Advertisement