19 replies to this topic

[JackBob]

so in a class I have
D3DMATERIAL9* mmaterial;

LPDIRECT3DTEXTURE9* ttexture;

as global variables that are used for loading in meshs and what not.

Whenever I close the program I get memory leaks from them as they are not being destroyed.
So simple enough I just make a destructor and in it I put
delete[] mmaterial;
delete[] ttexture;

inside of it
When I start up my game it crashes the game and visual studio. And I'm not particularly sure why...
my destructor looks like this
Ship::~Ship()
{
delete[] mmaterial;
delete[] ttexture;
}

Any insight or links to other articles would be helpful =)
or atleast what to search for since it seems apparently I can't find anything
Thanks

[GLForce]

I'd need more information on the crash. Does it pop an error message ? But on the top of my head I would say try using the Release() method for both objects instead of deleting them yourself. Of course you have to be sure they contain valid data before using the Release method.

[0x3a]

delete[] mmaterial;
delete[] ttexture;

Why this? Your data members are declared as:

D3DMATERIAL9* mmaterial;
LPDIRECT3DTEXTURE9* ttexture;

You do not have an array of them, just a pointer to them. Try to remove the brackets [ ].

[SiCrane]

You do not have an array of them, just a pointer to them. Try to remove the brackets [ ].

In C++ a pointer to a single element and a pointer to an array of elements have the same type. You can't know without seeing how it's used which a pointer is pointing to.

[GLForce]

You do not have an array of them, just a pointer to them. Try to remove the brackets [ ].

I agree for the first one (D3DMATERIAL9). However LPDIRECT3DTEXTURE9 is a definition of a pointer to a DIRECT3DTEXTURE9 structure. However I'm a bit rusty with C++ and I'm not sure how the delete operator works with typedefs

EDIT : Well, SiCrane must know better than me ;)

[Daft Code]

you cant delete a texture, its governed by the graphics adapter.
Use

delete[] mmaterial; (if its an array)
ttexture->Release();

instead.

or even better:

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

if(ttexture != NULL)
ttexture->Release();

with that, youll also have to set the pointers to null in the ctor:

Ship::Ship()
{
mmaterial = NULL;
ttexture = NULL;
}

with that, if something happens and the stuff never gets allocated, it wont crash since the pointer has not changed from NULL.

[0x3a]

You do not have an array of them, just a pointer to them. Try to remove the brackets [ ].

In C++ a pointer to a single element and a pointer to an array of elements have the same type. You can't know without seeing how it's used which a pointer is pointing to.

True, but seeing he had a crash and his data is, hopefully, correct it could be the case.

[Ectara]

In the case of some compilers sticking the length of an array allocated with new just before the returned pointer, wouldn't attempting to read before the block allocated for a single object cause an access violation if one uses the wrong delete?

[SiCrane]

However I'm a bit rusty with C++ and I'm not sure how the delete operator works with typedefs

The delete operator doesn't care about typedefs, it just looks at the actual type. In the case of an LPDIRECT3DTEXTURE9 *, that would be a Direct3DTexture9 **. This could either be a pointer to a Direct3DTexture9 pointer or a pointer to an array of pointers.

True, but seeing he had a crash and his data is, hopefully, correct it could be the case.

There is a difference between saying "this could be the cause of your crash" and "this is the cause of your crash". Please observe this distinction when making future posts.

In the case of some compilers sticking the length of an array allocated with new just before the returned pointer, wouldn't attempting to read before the block allocated for a single object cause an access violation if one uses the wrong delete?

In theory, it could. Using the wrong delete or delete [] has undefined behavior, which means anything can happen, including an access violation on that read. However, it's not guaranteed. For an access violation to occur on that read, the memory in that location would need to lack read permission, and, in practice, most memory allocators place other bookkeeping structures in front of allocated blocks, so that particular read rarely causes an access violation by itself. Acting on the information in the memory, on the other hand, has much better chances of bringing down the program somehow. If the type lacks a destructor, then it's entirely possible for using delete [] instead of delete to deallocate the memory successfully. Undefined behavior means anything can happen, up to and including "working".

[JackBob]

Ok first off Thanks for all the replies. I figure I should post all the code so you guys can have a better idea of what is happening

Ship::Ship(LPCTSTR nameT, LPDIRECT3DDEVICE9 dDevice)
{

d3Device = dDevice;
name = nameT;
LPD3DXBUFFER bufShipMaterial;[/size]

            D3DXLoadMeshFromX(name,    // load this file
                      D3DXMESH_SYSTEMMEM,    // load the mesh into system memory
                       d3Device,    // the Direct3D Device
                      NULL,    // we aren't using adjacency
                      &bufShipMaterial,    // put the materials here
                      NULL,    // we aren't using effect instances
       &NumMaterials,    // the number of materials in this model
                      &Mesh);    // put the mesh here[/size]

  // retrieve the pointer to the buffer containing the material information
    D3DXMATERIAL* tempMaterials = (D3DXMATERIAL*)bufShipMaterial->GetBufferPointer();[/size]

   // create a new material buffer for each material in the mesh
mmaterial = new D3DMATERIAL9[NumMaterials];
ttexture = new LPDIRECT3DTEXTURE9[NumMaterials];[/size]

     for(DWORD i = 0; i < NumMaterials; i++)    // for each material...
    {
        mmaterial[i] = tempMaterials[i].MatD3D;    // get the material info
        mmaterial[i].Ambient = mmaterial[i].Diffuse;    // make ambient the same as diffuse
  // if there is a texture to load, load it
  if(FAILED(D3DXCreateTextureFromFileA(d3Device,
           tempMaterials[i].pTextureFilename,
           &ttexture[i])))
       ttexture[i] = NULL;    // if there is no texture, set the texture to NULL
    }
}

Now that is my constructor for my Ship class. All it does really is just load in the mesh I want for whichever "Ship"
I realize some is messy and I intend on cleaning on all my code once I fix all my memory leaks.

and my destructor

Ship::~Ship()
{
for(DWORD i = 0; i < NumMaterials; i++)
{
  ttexture[i]->Release();
}

delete[] ttexture;
delete[] mmaterial;
}

and after reading through your comments and what not I tried using just delete vs delete[]. In my default constructor I made both variables null just in case

Pretty much what happens is when I run game in visual studio as its loading up it freezes and freezes up visual studio to the point where I have to end process everytime.
So i'm assuming there is a memory leak that its just not liking?

[rip-off]

Does your class have a copy constructor or assignment operator? Or rather, is it noncopyable or does it obey the rule of three?

Your constructor appears to lack appropriate error handling. You don't handle the case where mesh creation fails. You explicitly do handle the case where texture creation fails - but you don't account for it in your destructor. You should probably log failed texture creation. You probably want to release any resources associated with the mesh too.

[SiCrane]

and after reading through your comments and what not I tried using just delete vs delete[].

Please don't voodoo program. If you use new to allocate memory use delete to deallocate the memory. If you use new [] to allocate the memory use delete [] to deallocate the memory. Or, since it seems like you are using dynamic arrays here, use a std::vector instead of manually managing the memory and you don't need to worry about explicitly deallocating the memory.

[JackBob]

After putting in place the proper checks and what not into the constructor and destructor as soon as I run it it acts like its on a never ending loop that cause both the program and visual studio to freeze up. This has only ever happened to me when a memory leak has happened that I just completely missed. But this time I can't see anything I'm missing. If its a destructor why at run time would it cause this? My class does not obey the rule of three. After reading up on that I couldn't imagine that being the cause of this problem.

Thanks for the replies by the way guys
this problem is frustrating.

p.s. I tried the std:vector I was able to declare the variables. But for somereason I couldn't figure out how to set the variables. i.e. mmaterial = ? I tried looking it up online but so far nothing that even remotely shows it. Maybe my searching skills need improving.

[rip-off]

How did I miss this part:

so in a class I have
D3DMATERIAL9* mmaterial;

LPDIRECT3DTEXTURE9* ttexture;

as global variables that are used for loading in meshs and what not.

Which is it? Are these variables global, or class members?

If they are global, you should be initialising and cleaning them up outside the ship class. Otherwise multiple ship instances won't play nice together.

If its a destructor why at run time would it cause this?

Destructors run at run time. They don't run at compile time, which is the usual "opposite" of run time.

In case you though they only run when your program is shutting down, this is not the case. They run whenever an object goes out of scope, or is explicitly deleted.

void example()
{
    Ship value(...);
    Ship *pointer = new Ship(...);

    // Do stuff

    delete pointer; // pointer->~Ship() semi-implicitly called here

} // value.~Ship() implicitly called here

My class does not obey the rule of three. After reading up on that I couldn't imagine that being the cause of this problem.

It could be, if your class is being accidentally copied or assigned. Or if the values are global, each additional ship instance will clobber the previous values.

I tried the std:vector I was able to declare the variables. But for somereason I couldn't figure out how to set the variables. i.e. mmaterial = ? I tried looking it up online but so far nothing that even remotely shows it. Maybe my searching skills need improving.

Show us what you've tried.

[JackBob]

When I said global variables I meant as they are global inside the Ship class. So when I create a new object for the class each object should have its own variables right?

I'm thinking though, when I create the object in my main class i'm doing
Ship one = Ship(insertstuff here);

and thats it. I notice some people have pointers to there objects or something of the sort. I may have that all wrong.

now that I think about it the second ship seems textures itself red for no real reason. And I know the Bi plane has red on it.

and as for my attempt I did

std::vector<D3DMATERIAL9*> mmaterial;

then tried to define it doing

mmaterial = new D3DMATERIAL9(NumMaterials);
also
mmaterial = new D3DMATERIAL9[NumMaterials];
also
mmaterial = new <D3DMATERIAL9> and thats about as far as I got on that one.

I notice in all the examples they do
std::vector<D3DMATERIAL9*> mmaterial(NumMaterials);
I'm not sure how to break that up at all though

I know its got to be absolutely simple. But for the life of me I can't figure it out

thanks for your help rip-off

[mhagain]

For your original problem, you might want to double-check when your destructor actually runs. It sounds as though it's running after you've Released your IDirect3DDevice9 interface, which explains both the crashes and the leaks. http://blogs.msdn.com/b/oldnewthing/archive/2004/05/20/135841.aspx

(It's worth noting by the way that D3D uses COM interfaces, not C++ classes. They might look superficially similar but there is a whole world of subtle differences between them.)

For std::vector, you're going about it in a slightly convoluted manner. It's really as simple as this (untested, off the top of my head, don't copy/paste this!!!):

std::vector<LPDIRECT3DTEXTURE9> textures;
LPDIRECT3DTEXTURE9 newtexture = NULL;
Device->CreateTexture (blahblahblah... &newtexture...);
textures.push_back (newtexture);

for (int i = 0; i < textures.size (); i++)
   textures[i]->Release ();

textures.clear ();

[JackBob]

The program would still run correctly until shutdown time if it is being released after IDirect3DDevice9 wouldn't it? since my problem seems to be immediate on start up.

I got the vector compiling finally kinda

std::vector<LPDIRECT3DTEXTURE> ttexture;

// constructor method

LPDIRECT3DTEXTURE temp = NULL;

for loopy
createtexture(blahbluhblah... &temp);

ttexture.push_back(temp)

and it all works just fine and my texture memory leaks are now gone! YaY! Thanks mhagain
Although I'm still curious as to why my destructor hates me


also p.s. could anyone tell my whyD3DXCreateFont

creates a memory leak?

[mhagain]

A lot of D3DX interfaces (and ID3DXFont is one of them) create other resources behind the scenes for you. These may be vertex buffers, textures, whatever. You can tell this by checking if the D3DX interface has OnLostDevice and OnResetDevice members. In these cases, always call OnLostDevice before Releasing the interface otherwise you'll leak those resources.

[JackBob]

Ok I've finally got around to trying to implement OnLostDevice and OnResetDevice

and for some odd reason they are still leaking memory. And when I comment them out the memory errors are on so l must be doing something wrong.

i have a
bool checkDevice()
that i use when I do
if(checkDevice())
render();

so that way if something is wrong it wont try and render. I'm hoping I did that correctly and in this checkDevice its

bool checkDevice()

{

 switch(g_pd3dDevice->TestCooperativeLevel())

 {

  case D3DERR_DEVICELOST: 

   {

    onLostDevice();

    return false;

   }

  case D3DERR_DEVICENOTRESET:

   {

    onResetDevice();

    return false;

   }

 }

 return true;

}

now I know the device reset needs some work and that was the next thing to do but for the moment onLostDevice seems to not work
onLostDevice just has
dxfont->OnLostDevice();
and so on.
I tried just calling this method right as the loop is ending before I call my release method for everything and I'm still getting memory leaks. Any idea what I could be doing wrong?
Thanks for all the help
sprite->onLostDevice();

[mhagain]

You've just got them in the wrong place. The terminology used by D3D here can be a little confusing, so that's understandable.

The right place to call OnLostDevice is when you get D3DERR_DEVICENOTRESET; this means that the device was once lost, is not any more, but needs a reset before you can continue drawing. At that point you need to call your OnLostDevice methods, then Reset the device, then TestCooperativeLevel again until you get D3D_OK, at which point the reset has completed and you can call OnResetDevice and continue drawing.