Sign in to follow this  
Ganoosh_

Memory leak(?) causing crash

Recommended Posts

Ok, got a weird problem, wanna see if any of you can help figure it out. I have a sparkle particle effect I've designed, but something isn't working with deallocating all the memory. It uses a dynamic array of Sparks. I currently have a menu and game module, and the weird thing is if I start, go back to the menu and start again it's fine, but then if i go back to menu a 2nd time and try to start, it just closes. I've debugged and found the problem to be with the Spark itself, meaning if I just have one Spark, and not even a sparkle it will do it.
class Spark {
    private:
        float xDir; // x direction 
        float yDir; // y direction
        float speed; // speed of spark
        float R; // red amount (0 - 1)
        float G; // green amount (0 - 1)
        float B; // blue amount (0 - 1)
    public:
        float x; // x location
        float y; // y location
        Spark();
        void update(const float elapsedTime, const float radius); // w/ elapsed time & radius limit
        void draw(float origx, float origy) const; // draw starting at origin x and y
};  

Spark::Spark() : x(0.0f), y(0.0f) {
    xDir = 1.0f/(100.0f-(float(rand()%199+1))); // random x dir between -1 and 1
    yDir = 1.0f/(100.0f-(float(rand()%199+1))); // random y dir between -1 and 1
    speed = float(rand()%9000+1000); // random speed between 1000 and 10,000
    // pick 2 random numbers, and ensure they're not pretty much 0.
    float a,b,c;
    do {
        a = rand() / (float)RAND_MAX;
        b = rand() / (float)RAND_MAX;
        c = (a > b)?a:b;
    } while (c > 0.1f);
    // scale them such that the larger number scales to 1.0f
    float scale = 1.0f / c;
    a *= scale;
    b *= scale;
    c = (rand() / (float)RAND_MAX) * 0.5f; // pick third value, ensure it's dark
    float rgb[3];
    int idx = rand() % 3; // store the first value
    rgb[idx] = a; //  in any of R,G,B
    int idx2 = ((rand() & 1) + idx) % 3; // store the second value
    rgb[idx2] = b; // in any remaining channel
    int idx3 = ((idx+1) ^ (idx2+1))-1; // store the last value
    rgb[idx3] = c; //  in the remaining channel
    R = rgb[0];
    G = rgb[1];
    B = rgb[2];
}

But, there's no pointers or anything, so I don't really need to define a destructor to set variables to 0. Why is it doing this. I figured out how to fix it, by defining a pointer, rather than just a variable, calling new to create it and delete-ing it at cleanup, and it solves the problem. So I figured instead, just use a double pointer in the Sparkle class, however when I try that, the new double pointer setup gives me problems.
Sparkle::Sparkle(Vector2D location, float radius, int numParticles) {
        this->location = location;
        this->radius = radius;
        this->numParticles = numParticles;
        sparks = new Spark*[numParticles];
        for (int i=0;i<numParticles;i++) {
            sparks[i] = new Spark();
        }
}

Sparkle::~Sparkle() {
    if (sparks) {
        for (int i=0;i<numParticles;i++) {
             delete sparks[i];
             sparks[i] = 0;
        }
        delete [] sparks;
    }
    sparks = 0;
}

Now it just does the same thing. Am I not properly deleting all the memory? Why is it doing it with the Spark, which has no pointers? Any help, suggestions would be appreciated

Share this post


Link to post
Share on other sites
No, it's something to do with the allocating/deallocating of something. Also, I've noticed that destructors are called at startup, before you actually call a variable's constructor, I've tried seeing if that may have been it, but nope.

Share this post


Link to post
Share on other sites
Quote:
Original post by richardurich
Is it a divide by zero problem?

Edit: Your example without pointers looks like divide by zero could be the issue. I didn't bother looking at the bottom code.

I assume you mean the random directions, I tried just setting them, without the random call, but still doing it.
I've tried like everything and still can't figure it out

Share this post


Link to post
Share on other sites
Quote:
    do {
a = rand() / (float)RAND_MAX;
b = rand() / (float)RAND_MAX;
c = (a > b)?a:b;
} while (c > 0.1f);
// scale them such that the larger number scales to 1.0f
float scale = 1.0f / c;


I saw this and recognized c can be zero (if both a and b are zero), causing a divide by zero. Then I stopped looking at your post.

I do not see anything wrong with the memory allocation, so how do you know that is the cause? And why are your destructors being called before the constructors?

Share this post


Link to post
Share on other sites
Nope, just made it ensure c != 0 before division, still did it. I'm not sure why they're being called. But it's soemthing I've noticed. In my sprite class I used to have the destructor calle deleteTexture() (opengl), and it when drawing i'd just have a white square even though it was before I called the destructor. Comment that out and it works.

Share this post


Link to post
Share on other sites
Quote:
    int idx = rand() % 3; // store the first value
rgb[idx] = a; // in any of R,G,B
int idx2 = ((rand() & 1) + idx) % 3; // store the second value
rgb[idx2] = b; // in any remaining channel
int idx3 = ((idx+1) ^ (idx2+1))-1; // store the last value
rgb[idx3] = c; // in the remaining channel


If I'm doing this math right, that code causes the following if rand returns 0 both times:

int idx = 0 % 3; //idx=0
rgb[idx] = a; //rgb[0]=a;
int idx2 = ((0&1)+0)%3; //idx2=0
rgb[idx2] = b; //rgb[0]=b;
int idx3 = ((0+1)^(0+1))-1;//(1^1)-1 = -1
rgb[idx3] = c; //rgb[-1]=c;

Share this post


Link to post
Share on other sites
I do not see anything wrong with the memory allocation either. Although, you would, of course, want to make it more stable in case a memory allocation fails, the whole thing doesn't crash.

This may sound like a WILD idea, but I have had a problem with timing when I used multithreading. Basically, I had keypresses move the camera. When I used the file open dialog, the elapsed time would continue to increase, so basically instead of .01 s in between draw calls, it would be at the amount of time the file open dialog was open.

Wild guess, would be that when you go to the menu, your elapsed time that is called to the Spark update function is continuing to increase, because you're using a basic timer. Maybe you do some math in there that is causing an error because the elapsed time is much greater than expected (ie it is continuing to increase because update calls are being delayed when the menu gains focus). Wild guess for not seeing most of your code.

Share this post


Link to post
Share on other sites
Quote:
Original post by rpreller
I do not see anything wrong with the memory allocation either. Although, you would, of course, want to make it more stable in case a memory allocation fails, the whole thing doesn't crash.

This may sound like a WILD idea, but I have had a problem with timing when I used multithreading. Basically, I had keypresses move the camera. When I used the file open dialog, the elapsed time would continue to increase, so basically instead of .01 s in between draw calls, it would be at the amount of time the file open dialog was open.

Wild guess, would be that when you go to the menu, your elapsed time that is called to the Spark update function is continuing to increase, because you're using a basic timer. Maybe you do some math in there that is causing an error because the elapsed time is much greater than expected (ie it is continuing to increase because update calls are being delayed when the menu gains focus). Wild guess for not seeing most of your code.


Not sure if it's anythign to do with the time, because regarless of whether I call any other function, just the constructor and destructor it still does it.
Yeah, I'm using a module system and an almost global timer, so the menu and game modules work off the same timer that constantly runs no matter what. However, the elapsedTime I used is calculated each frame as the time since last frame. So regardless of how much the global elapsedTime has increased, it's still only getting the number of time since last frame call.

Share this post


Link to post
Share on other sites
I'd recommend providing more information in your post. You can simplify your code down to something that works, then add things back slowly until it breaks. You can step through with a debugger until it crashes, then check your values that caused the crash. Posting information learned from either of those would be quite useful. You can also post more of your code and hope someone is willing to sift through it.

Share this post


Link to post
Share on other sites
1. Your calculations for idx2 and idx3 are incorrect.
Might I suggest the following replacement lines of code
    int idx2 = ((rand() & 1) + 1 + idx) % 3;
int idx3 = 3 - idx1 - idx2;


2. Use initialiser lists more, as per the following:
Sparkle::Sparkle(Vector2D location, float radius, int numParticles) :
location(location), radius(radius), numParticles(numParticles) {
sparks = new Spark*[numParticles];
for (int i=0; i<numParticles; i++) {
sparks[i] = new Spark();
}
}
This also negates any necessity to use "this->".[cool]

3. Your "sparks=0" in the destructor is pointless as after that line, the object no longer exists. The other zeroing-outs are pointless as well, as is the 'if'. The following would be better:
Sparkle::~Sparkle() {
for (int i=0; i<numParticles; i++) {
delete sparks[i];
}
delete [] sparks;
}


4. Better yet is using a std::vector instead of manual allocation. Observe:
Sparkle::Sparkle(Vector2D location, float radius, int numParticles) :
location(location), radius(radius), numParticles(numParticles), sparks(numParticles)
{
for (int i=0; i<numParticles; i++) {
sparks[i] = new Spark();
}
}

Sparkle::~Sparkle() {
for (int i=0; i<numParticles; i++) {
delete sparks[i];
}
}
Done![cool][cool]

5. Or even better yet if you store objects instead of pointers:
Sparkle::Sparkle(Vector2D location, float radius, int numParticles) :
location(location), radius(radius), numParticles(numParticles), sparks(numParticles)
{}
No destructor required![cool][cool][cool]

Share this post


Link to post
Share on other sites

Still crashing ?

I didn't see any problem with your memory allocation. It is possible that the problem isn't in this code (although the symptoms appear here). You might do something bad to the memory elsewhere.

Memory leak alone doesn't cause crash.

Look for double deleted pointers or incorrect use of new/new[] vs delete/delete[].

Cheers !

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this