3. Merkmal einer Klasse lässt sich nicht löschen bzw. static return value - memory leak

Merkmal einer Klasse lässt sich nicht löschen bzw. static return value - memory leak

  • S

    Hallo,

    ich bin wiedermal auf Hilfe angewiesen 😕

    Vielleicht ist die Überschrift etwas verwirrend - das Problem ist folgendes. Wie schon mal erwähnt, das gesamte Programm ist eine OpenGL Anwendung mit GLUT und C++ und ich bin relativ neu in all den drei Sachen.
    Ich hatte gerade ein riesiges(!) Memory leak (1GB in 20sec), das ich am eindämmen bin. Hat fast geklappt bis auf folgendes:
    Ich habe eine Mathe Klasse. diese enthält folgende Methode:

    //mathpg.h
static float* VertexAddition( float* Vertex1, float* Vertex2 );

//mathpg.cpp
float* MathPG::VertexAddition( float* Vertex1, float* Vertex2 ) 
{
	int DimensionOfVertex = 4;
	float* VertSum = new float[DimensionOfVertex];

	for ( int i = 0; i < DimensionOfVertex - 1; i++ )
	{
		VertSum[i] = Vertex1[i] + Vertex2[i];
	}
	VertSum[3] = 1.0f;

	return VertSum;
}

    Diese benutze ich nun bei der Berechnung hier:

    inBall->SetBallMovement( MathPG::VertexAddition( inBall->MovementVertex, Gravity ) );

    Hier wird also eine Methode der Klasse Ball aufgerufen, die pro Frame die neue Position des Balls berechnen soll und diesen darstellt.

    void Ball::SetBallMovement( float* inMovement) 
{
	//delete [] MovementVertex;
	for (int i = 0; i < DimensionOfVertecies; i++ )
	{
		MovementVertex[i] = inMovement[i];
	}
	//MovementVertex = inMovement;
//	delete [] inMovement;

	Ball::UpdateBall( );
}

    Mein Annahme ist, dass in der static Methode der Mathe Klasse jedes mal ein neues float Array auf dem Heap erstellt wird und letztendlich dessen Pointer in der SetBallMovement Methode ankommt.
    MovementVertex ist eine Eigenschaft der Klasse Ball.
    Ich gehe davon aus, dass der MovementVertex schon einen Speicherbereich im Heap belegt und ich nun durch MovementVertex = inMovement; den Pointer zu diesem existierenden reservierten Heapstück verlieren würde und durch den Pointer zu dem in der statischen Methode der Mathe Klasse erstellen Array ersetze. Also wollte ich den alten MovementVertex löschen, damit das Programm nicht leakt. Das geht aber nicht! Ich bekomme eine Fehlermeldung die mich höchstwahrscheinlich darauf hinweisen soll, dass ich den MovementVertex nicht löschen darf.
    Also dachte ich mir, versuche ich es mit einem "Hack", falls ich keine Merkmale eines Objektes löschen darf, während das Objekt existiert oder sowas und kopiere das Array aus der Mathe Klasse in den Heapspeicher der für das eigentliche Merkmal reserviert ist. Wenn ich das tue und danach das Array aus der Mathe Klasse lösche geht das auch nicht! Selber Fehler...

    Nun weiß ich nicht, wo mein Denkfehler ist?!
    Hat jemand eine Idee?

    0
  • ?

    wirklich sehr, sehr furchtbarer code.
    ungetestet:

    #include <vector>

typedef std::vector<float> float_vector;
float_vector MathPG::VertexAddition( const float_vector& Vertex1, const float_vector& Vertex2)
{
    enum { DIMENSIONS = 4 };
    assert(Vertex1.size() == DIMENSIONS && Vertex2.size() == DIMENSIONS);
    float_vector result(DIMENSIONS);

    for ( int i = 0; i < DIMENSIONS - 1; i++ )
    {
        result[i] = Vertex1[i] + Vertex2[i];
    }
    result[3] = 1.0f;
    return result;
}

    das ist zwar immer noch weit von einer effizienten implementierung entfernt, behebt aber zumindest die gröbsten schnitzer. und leakt nicht.

    die dimensionen deines vektors sollten global definiert sein. ich empfehle außerdem eine eigene klasse, die ein paar operatoren implementiert (operator+, operator-).

    0
  • S

    geschockt schrieb:

    wirklich sehr, sehr furchtbarer code.
    ungetestet:

    #include <vector>

typedef std::vector<float> float_vector;
float_vector MathPG::VertexAddition( const float_vector& Vertex1, const float_vector& Vertex2)
{
    enum { DIMENSIONS = 4 };
    assert(Vertex1.size() == DIMENSIONS && Vertex2.size() == DIMENSIONS);
    float_vector result(DIMENSIONS);

    for ( int i = 0; i < DIMENSIONS - 1; i++ )
    {
        result[i] = Vertex1[i] + Vertex2[i];
    }
    result[3] = 1.0f;
    return result;
}

    das ist zwar immer noch weit von einer effizienten implementierung entfernt, behebt aber zumindest die gröbsten schnitzer. und leakt nicht.

    die dimensionen deines vektors sollten global definiert sein. ich empfehle außerdem eine eigene klasse, die ein paar operatoren implementiert (operator+, operator-).

    Ok... furchbarer Code also... Naja... Ich habe vor 4 Wochen mit C++ angefangen. Außerdem ist der Code den ich im ersten Post reingestellt habe auf 3 Klassen verteilt. Das eine ist die Mathe Klasse, die Vektoren skaliert, ihre Normalen findet, subtrahiert, addiert usw. Das zweite ist eine Physik Klasse die für Gravitation und Kollisionsdetektion zuständig ist und das dritte ist die Klasse für den Ball.

    So - zu deinem Code:

    typedef std::vector<float> float_vector;
    - ich habe gelesen, ein vector ist nichts anderes als ein dynamisches Array. Was du da mit den < > Klammern machst kenne ich nicht. Ich nehme an, dass heisst, "ein vector der den Typ float enthält".

    float_vector MathPG::VertexAddition( const float_vector& Vertex1, const float_vector& Vertex2)
    - was bringt mir das const vor den Parametern? Und was bringt es mir anstatt der Pointer eine Referenz zu nehmen? Klar, durch const kann es nicht innerhalb der Funktion geändert werden und der Referenz kann im Gegensatz zum Pointer kein neues Ziel zugewiesen werden. Aber ich weiß es einfach nicht was das in meinem Fall für einen Unterschied macht - also bitte klärt mich auf.

    enum { DIMENSIONS = 4 };
    - ist klar!

    assert(Vertex1.size() == DIMENSIONS && Vertex2.size() == DIMENSIONS);
    - hab ich gerade nachgelesen, macht sinn. Aber ich habe bisher nicht die Zeit das Programm Idiotensicher zu schreiben, bin froh wenn es läuft und nicht leakt. Wenn ich vor der Abgabe noch Zeit finde werde ich solche Checks sicher noch einbauen.

    float_vector result(DIMENSIONS);
    - Du allokierst den Speicher für den vector auf dem Stack, richtig? Und dadurch willst du das Leak umgehen? Habe ich auch schon versucht. Aber genau das ist ja das Problem. Der vector scheint auf dem Stack wieder gelöscht zu werden, bevor ich ihn brauche. Deshalb schicke ihn in ja auf den Heap mit "new". Ist das so falsch? Und warum?

    Meine Hauptfrage, warum ich keins der Array löschen kann ist damit leider nicht beantwortet. Aber danke dir trotzdem!
    Ach und mit Klassen für Operatoren definieren meinst du, dass ich danach vectoren mit + und - addieren bzw. subtrahieren kann?
    Das wäre sicher nen cooles feature! Aber da müsste ich mir ja erst anschauen wie ich die Operatoren überlade...

    0
  • F

    Wenn du es unbedingt schnell haben willst (also die Lösung, nicht die Performance) und ohne Leak:

    float *sum = MathPG::VertexAddition( inBall->MovementVertex, Gravity );
inBall->SetBallMovement( sum );
delete[] sum;
    0
  • D

    Ersteinmal würde ich dir eher empfehlen dich noch ein wenig länger mit den Grundlagen zu beschäftigen, da die in C++ sehr wichtig sind und du da kaum drum herum kommst.

    Aber zu deinem Problem:
    Das Problem ist ja, dass du eine Objekt haben willst, dass deinen Vector entspricht. Und du gibst da einen Zeiger auf ein intern erstelltes Objekt zurück, was an sich schon mal schreklich ist, da du somit die Verantwortung an den Aufrufer übergibst, was fatal sein kann, wenn er das nicht weiss.
    Ich würde da eher empfehlen eine Klasse Vector zu schreiben, die Kopiersemantik anbietet und du dann intern ein Objekt erstellst und das dann per Kopie nach aussen übergibst. (Schnelle Variante geht da mit std::vector, ist aber ein wenig umständlich, da es nicht einen mathematischen vector entspricht und somit auch keine Operationen dafür anbietet). Ist aber sicher besser, als das per Array zu lösen.

    Das mit den <> ist ein template der Standardbibliothek. Nämlich std::vector. Und wie du richtig erkannt hast ergibt das dann einen vector, dass den Typ, den du angibst enthälst.

    float_vector result(DIMENSIONS);
    - Du allokierst den Speicher für den vector auf dem Stack, richtig? Und dadurch willst du das Leak umgehen? Habe ich auch schon versucht. Aber genau das ist ja das Problem. Der vector scheint auf dem Stack wieder gelöscht zu werden, bevor ich ihn brauche. Deshalb schicke ihn in ja auf den Heap mit "new". Ist das so falsch? Und warum?

    Richtig. In deinem Fall passiert das. Aber nur deshalb, weil ein Array keine Kopiersemantik anbietet und somit lediglich ein Zeiger zurückgibst und der Speicher am Ende der Funktion wieder freigegeben wird. geschockt benutzt aber einen std::vector, der sozusagen ein handliches Array ist und welcher auch das Kopieren erlaubt.

    Meine Hauptfrage, warum ich keins der Array löschen kann ist damit leider nicht beantwortet.

    Ich verstehe nicht ganz, was du damit meinst..

    int var[5]; //Array von 5 ints. Du musst dich da nicht drum kümmern, wann es freizugeben ist. Das wird automatisch am Ende des Scopes gemacht. (Sprich, wenn das nächste } kommt (normalerweise Ende einer Funktion)

int* pVar = new int[5]; //du erzeugst ein Array von 5 ints dynamisch und lässt pVar auf den Anfang zeigen.

// delete[] pVar; // So wäre das richtig
pVar = 0; // upps du hast den Zeiger verloren und kannst den Speicher somit nicht mehr freigeben..

    In etwa das?

    0
  • S

    Hallo!

    Erstmal herzlichen Dank für die Antworten!!!

    @Fellhuhn: Deinen Vorschlag habe ich schon umgesetzt. Danke dir.

    @drakon: Welche Art von Grundlagen denkst du sollte ich denn WIE vertiefen? Mir ist das auch schon aufgefallen, dass C++ deutlich "anspruchsvoller" als z.B.: C# ist, womit ich vorher gearbeitet habe.
    Liegt ja schon am nicht vorhandenen Heap-Garbage-Collector. Die "was sollte man über C++ wissen" Seite dieses Forums habe ich schon gelesen.
    Das mit <> und den Templates habe ich inzwischen schon gelesen. Aber danke!

    Zu der eigenen Klasse:
    Sagen wir, ich erstelle eine Klasse, die meinen Vektor als float Array enthält und eben die Kopierfunktion bietet. Wenn ich in der Mathe Klasse nun einen Objekt dieser Klasse instanziere, und das dann per Kopiermethode zurückgebe, wird dann das in der Mathe Klasse erstellte Objekt automatisch nach eine der Mathe Additionsmethode gelöscht? Oder muss das in der Kopierfunktion stattfinden? Könntest du ganz eventuell mal kurz die Kopier Methode umreissen?

    Ansonsten habe ich nun das Problem, dass Fellhuhn's Weg zwar in dem Fall funktioniert, aber in meinem zweiten Fall in dieser Methode der Klasse Ball (siehe unten) komischer weise nicht (darum ist das delete auskommentiert). Wenn ich das delete [] dort ausführe bekomme ich "Debug Assertion Failed" "Block-Type-is-Valid". Hab ich mal bei Google nachgeschlagen, liegt normalerweise daran, dass ich etwas lösche das schon gelöscht ist?!
    Habt ihr eine Ahnung warum es geht, wenn es sich nicht um eine Eigenschaft eines Ohjektes handelt, aber im anderen Fall diesen Fehler ausgibt?

    void Ball::UpdateBall( void )
{
        float* sum = MathPG::VertexAdditionStack( Position, MovementVertex );	

	for (int i = 0; i < 4; i++ )
	{
		Position[i] = sum[i];
	}
	Ball::DrawBall( );
	//delete [] sum;    //gibt
}
    0
  • D

    @drakon: Welche Art von Grundlagen denkst du sollte ich denn WIE vertiefen? Mir ist das auch schon aufgefallen, dass C++ deutlich "anspruchsvoller" als z.B.: C# ist, womit ich vorher gearbeitet habe.
    Liegt ja schon am nicht vorhandenen Heap-Garbage-Collector. Die "was sollte man über C++ wissen" Seite dieses Forums habe ich schon gelesen.
    Das mit <> und den Templates habe ich inzwischen schon gelesen. Aber danke!

    Zu der eigenen Klasse:
    Sagen wir, ich erstelle eine Klasse, die meinen Vektor als float Array enthält und eben die Kopierfunktion bietet. Wenn ich in der Mathe Klasse nun einen Objekt dieser Klasse instanziere, und das dann per Kopiermethode zurückgebe, wird dann das in der Mathe Klasse erstellte Objekt automatisch nach eine der Mathe Additionsmethode gelöscht? Oder muss das in der Kopierfunktion stattfinden? Könntest du ganz eventuell mal kurz die Kopier Methode umreissen?

    Naja. Allgemein halt. Den Umgang ein wenig mit der Sprache. Aber das kommt eigentlich mit der Zeit.

    Ich persönlich würde da ganz einfach eine Klasse machen, die 4 floats enthält (kein Array) und dann fällt die Kopierarbeit auch gleich weg, weil das richtig kopiert wird.

    class Vector
{
 // c'tor
 float x;
 float y;
 float z;
 float a;
 .. //Operatoren Überladung usw.
};

    Vector a;
// mach aus a was spezielles
Vector b = a; // x,y,z,a wird kopiert.
    0
Anmelden zum Antworten