Speicherleck nur beim "new"?

Bashar

david.dda schrieb:

Ich habe noch einige Unklarheiten: warum erstellt man mit einem Zeiger direkt ein Objekt?

Du meinst, warum man int* p = new int; schreibt? Naja, du kannst auch erst den Zeiger definieren und dann zuweisen: int *p; p = new int; aber aus einer Reihe von guten Gründen bevorzugt man die Initialisierung.

Ich glaubte, ein Objekt wird erstellt (z.B. Person p=new Person(); ) und der Zeiger zeigt nur darauf hin (z.B. Person *z = &p; ).

Wo hast du denn das her? Das erste funktioniert ja schonmal gar nicht. Wenn du Person p = ... schreibst, deklarierst du p als Objekt vom Typ Person . new Person() erstellt aber ein neues Objekt vom Typ Person auf dem Freispeicher und liefert die Adresse dieses Objekts (d.h. einen Zeiger darauf zurück). Du kannst aber den Zeiger nicht an p zuweisen, p ist ja kein Zeiger.

Was ist der Sinn des Erzeugens eines Objektes mit einem Zeiger? - (z.B. Person *neu = new Person(); ) - bedeutet diese Initialisierung, dass ein Objekt Person erzeugt wird, irgendwo im Speicher abgelegt wird und nur durch den Zeiger erreicht werden kann?

Genau, und noch mehr: Das erzeugte Person-Objekt bleibt solange gültig, bis es mit delete zerstört wird. Die Alternative zum Verwenden von new (die du immer wieder falsch wiedergibst, da muss ich jetzt auch mal dringend zu einem vernünftigen Lehrbuch raten) funktioniert so:

Person p; // ein Objekt vom Typ Person wird erzeugt und p genannt

Diese Objekte werden zerstört, wenn der Gültigkeitsbereich von p verlassen wird, deshalb nennt sich das auch automatische Speicherdauer:

int main() {
  Person p; // nix new
  p.bla();
} // hier hört p auf zu existieren, es wird automatisch der Destruktor aufgerufen

Sebastian Pizer

david.dda schrieb:

Person p = new Person();

Nur so auf Verdacht: C++ != Java. Diese Sprachen bitte nicht durcheinander werfen. Was die Speicherverwaltung, Erzeugung von "Objekten", "Referenzen" und das Objekt-Modell angeht, unterscheiden sich diese Sprachen gewaltig.

david.dda

Ich habe schon einige Bücher über C++ gelesen, aber weil es nur viel Theorie ohne Praxis war, versteht man vieles nicht so eindeutig. Jedenfalls werde ich mir noch das Buch besorgen. Was ich fragen wollte:
Hier im Quellcode wird ein Zeiger pData erstellt. Dieser zeigt auf einen Struct, der im zweiten Teil abgebildet ist.

case USERCONTROL_CONNOP_TRANSFEROFFSETS:
			{
				t_connectiondata_transferoffsets* pData = (t_connectiondata_transferoffsets*)pConnOp->data;
				buffer = pData->pData;
				len = pData->len;

				unsigned char* p = buffer + 2;
				int* userid;
				__int64* offset;
				while ((p - buffer + 12) <= len)
				{
					userid = (int*)p;
					offset = (__int64*)(p + 4);
					t_connectiondata& data = m_UsersList[*userid];
					data.currentOffset = *offset;

					p += 12;					
				}
				// DA SOLLTE EINGEFÜGT WERDEN: delete pData;
			}
			break;
		}

Struct für pData

typedef struct
{
	void *data;
	int op;
	int userid;
} t_connop;

pConnOp (dass pData definiert: t_connectiondata_transferoffsets* pData = (t_connectiondata_transferoffsets*)pConnOp->data;) wird vom dem Eingabeparameter in der gleichen Methode übernohmen, wobei LPARAM ein _W64 long ist:

LRESULT CServer::OnServerMessage(CServerThread* pThread, WPARAM wParam, LPARAM lParam)
{
t_connop *pConnOp = reinterpret_cast<t_connop*>(lParam);
...

und die Methode OnServerMessage wird so verwendet:

std::list<CServerThread::t_Notification> notifications;
			pThread->GetNotifications(notifications);
			for (std::list<CServerThread::t_Notification>::const_iterator iter = notifications.begin(); iter != notifications.end(); iter++)
				if (pServer->OnServerMessage(pThread, iter->wParam, iter->lParam) != 0)
					break;

Ich kann hier also keinen Grund finden, warum pData wirklich gelöscht werden sollte, da kein Objekt erzeugt wird.

Sebastian Pizer

david.dda schrieb:

Ich habe schon einige Bücher über C++ gelesen,

welche?

david.dda schrieb:

Jedenfalls werde ich mir noch das Buch besorgen.

welches?

Sorry, aber Deine Glaubwürdigkeit ist, wegen dem, was Du so von Dir gibst, nahe Null. Ein Tipp für's nächste mal: Wenn Du demnächst irgendwo nochmal eine Frage stellst, könntest Du auch dein Erfahrungs- und Wissensstand verraten. Diese Info kann nie schaden und erleichtert es womöglich den Hilfsbereiten deine Fragen besser einzuordnen.

david.dda schrieb:

Was ich fragen wollte:
[...code_fragment...]
Ich kann hier also keinen Grund finden, warum pData wirklich gelöscht werden sollte, da kein Objekt erzeugt wird.

Das kann man gar nicht anhand dieses kleinen Code-Fragments beurteilen. Man müsste schon die Zusammenhänge kennen. Wo ist das dazugehörige new? Was hat sich der Programmierer dabei gedacht? Ist es irgendwo dokumentiert, wie die Speicherverwaltung der Puffer in diesem Kontext gedacht ist? u.s.w.

Gruß,
SP

Th69

Du mußt noch weiter im Code suchen, nämlich da, wo die Elemente der Liste 'std::listCServerThread::t_Notification' erzeugt werden... und dort wird es wohl eine Speicherallokation für 'lParam->data' geben.
(ist aber alles andere als schöner Code, da wären shared_pointer sicherlich besser - außerdem ist das m.E. die falsche Stelle: warum wird das nicht im Destruktor von CServerThread::t_Notification gemacht???)

david.dda

danke für den Hinweis, der "iterator" hat die Liste "notifications" durchgelaufen:

std::list<CServerThread::t_Notification> notifications;
			pThread->GetNotifications(notifications); //da muss die Speicherallokation für 'lParam->data' sein
			for (std::list<CServerThread::t_Notification>::const_iterator iter = notifications.begin(); iter != notifications.end(); iter++)
				if (pServer->OnServerMessage(pThread, iter->wParam, iter->lParam) != 0)
					break;

Die Liste wurde mit "GetNotifications(notifications)" erstmals gefüllt (dabei wurde der Inhalt dieser Liste mit der Liste "m_pendingNotifications" umgetauscht:

void CServerThread::GetNotifications(std::list<CServerThread::t_Notification>& list)
{
	EnterCritSection(m_threadsync);

	m_pendingNotifications.swap(list);

	if (m_throttled)
		SetPriority(THREAD_PRIORITY_NORMAL);

	LeaveCritSection(m_threadsync);
}

und die Liste "m_pendingNotifications" wurde wiederum so gefüllt:

void CServerThread::SendNotification(WPARAM wParam, LPARAM lParam)
{
	EnterCritSection(m_threadsync);
	t_Notification notification;
	notification.wParam = wParam;
	notification.lParam = lParam;

	if (m_pendingNotifications.empty())
		PostMessage(hMainWnd, m_nNotificationMessageId, 0, 0);

	m_pendingNotifications.push_back(notification);

	// Check if main thread can't handle number of notifications fast enough, throttle thread if neccessary
	if (m_pendingNotifications.size() > 200 && m_throttled < 3)
	{
		SetPriority(THREAD_PRIORITY_IDLE);
		m_throttled = 3;
	}
	else if (m_pendingNotifications.size() > 150 && m_throttled < 2)
	{
		SetPriority(THREAD_PRIORITY_LOWEST);
		m_throttled = 2;
	}
	else if (m_pendingNotifications.size() > 100 && !m_throttled)
	{
		SetPriority(THREAD_PRIORITY_BELOW_NORMAL);
		m_throttled = 1;
	}

	LeaveCritSection(m_threadsync);
}

Diese Methode "SendNotification" wurde dann mehrmals benutzt, hier ist ein Beispiel:

t_connop *op = new t_connop;
	op->data = 0;
	op->op = USERCONTROL_CONNOP_REMOVE;
	op->userid = m_userid;
	m_pOwner->SendNotification(FSM_CONNECTIONDATA, (LPARAM)op);

Es wurde also ein Zeiger "op" erstellt, der einen struct mit new erzeugt:

typedef struct
{
	void *data;
	int op;
	int userid;
} t_connop;

Deswegen musste dann dieses neues struct-Objekt, an dem "op" Zeiger hinweist, gelöscht werden. Ich hoffe, es stimmt so.

Antwort zu der vorigen Frage:
Ich habe C++Tutorial.pdf (ca. 140 Seiten) und Teach Yourself C++ in 21 days (da habe ich einige Teile übersprungen) und dann noch ziemlich viel Online, gelesen (zuletzt tutorial vom schornboeck nochmals über die Zeiger und dynamische Speicherreservierung). Ich habe mich ca. 2 Jahre privat mit Java beschäftigt und schon auch einiges programmiert (z.B. einen Tetris Spiel) und seit einem halben Jahr mit C++, aber eher viel theoretisch und nur mal sehr kleine Applikation ausprobiert - wie einen Taschenrechner mit Hilfe von Visual Studio. Deswegen scheinen Dir meine Fragen vielleicht so primitiv. In einem halben Jahr bin ich sicher wieder bißchen weiter.

Mizar

david.dda schrieb:

Antwort zu der vorigen Frage:
Ich habe C++Tutorial.pdf (ca. 140 Seiten) und Teach Yourself C++ in 21 days (da habe ich einige Teile übersprungen) und dann noch ziemlich viel Online, gelesen (zuletzt tutorial vom schornboeck nochmals über die Zeiger und dynamische Speicherreservierung). Ich habe mich ca. 2 Jahre privat mit Java beschäftigt und schon auch einiges programmiert (z.B. einen Tetris Spiel) und seit einem halben Jahr mit C++, aber eher viel theoretisch und nur mal sehr kleine Applikation ausprobiert - wie einen Taschenrechner mit Hilfe von Visual Studio.

Da ich nicht besonders viel von C++-Onlinetutorials halte, würde ich dir auch zu einem gescheiten Buch wie z. B. dem C++-Primer raten. Weil du ja auch folgendes geschrieben hattest:

david.dda schrieb:

Ich habe schon einige Bücher über C++ gelesen, aber weil es nur viel Theorie ohne Praxis war, versteht man vieles nicht so eindeutig.

würde ich dir das Buch auch deswegen empfehlen, weil du dort Unmengen an Übungsaufgaben findest an denen du dich versuchen kannst.

PS: Wer weiß, wenn du deine Eltern/Verwandten/Bekannten darauf ansprichst, dann kriegst du es vielleicht als Weihnachtsgeschenk. :xmas1:

Th69

Hallo david.dda,

die Freigabe des Speichers bezieht sich aber auf eine evtl. Zuweisung von "op->data = static_cast<void *>(new X);". Gibt es auch solche (oder ähnliche) Speicherallokationen?

Aber wie schon geschrieben, ist das keine gute Implementierung, anscheinend nur ein Notbehelf, um Speicherlecks zu fixen...
Edit: und auch sehr fehleranfällig, falls z.B. konstante Zuweisungen á la

op->data = (void *)42;
// oder
op->data = (void *)"hello";

passieren (habe extra mal kein static_cast benutzt, um zu zeigen, was alles so programmiert werden könnte...)

david.dda

danke, es stimmt, es wurde eine Allokation erstellt, die dann mit dem vergessenen delete statement nachträglich korrigiert wurde (es wurde also der reservierte Speicher an dem op->data zeigt, gelöscht - in diesem Fall der Speicher an dem "conndata" zeigt - ):

t_connectiondata_changeuser *conndata = new t_connectiondata_changeuser;
	t_connop *op = new t_connop;
	op->data = conndata;
	op->op = USERCONTROL_CONNOP_CHANGEUSER;
	op->userid = m_userid;
	conndata->user = m_status.user;

	m_pOwner->SendNotification(FSM_CONNECTIONDATA, (LPARAM)op);

	return TRUE;
}

*conndata ist ein Struct:

struct t_connectiondata_changeuser
{
	CStdString user;
};

nun kann ich aber nicht rausfinden, wo auch der Speicher an dem *op zeigt gelöscht wurde. Es sieht so aus, als würde dieser nicht deallokiert.

Mein Source code analyzer meldet folgendes:
*Memory Leak
(Code Quality, Control flow)

The function SendTransferinfoNotification() in ControlSocket.cpp allocates memory on line 2847 and fails to free it.*

Line 2847 enthält folgendes Code: t_connop *op = new t_connop;

Sebastian Pizer

david.dda schrieb:

nun kann ich aber nicht rausfinden, wo auch der Speicher an dem *op zeigt gelöscht wurde. Es sieht so aus, als würde dieser nicht deallokiert.

Auch, wenn Du das als Frage formuliert hättest, kann man Dir hier nicht helfen, weil wir "Dein Analyse-Tool" nicht kennen und keinen Überblick über das Programm haben, was Du analysierst. Du anscheinend auch nicht.

david.dda schrieb:

Mein Source code analyzer

Ja welcher ist dass denn nun?!

david.dda schrieb:

meldet folgendes:
*Memory Leak
(Code Quality, Control flow)

The function SendTransferinfoNotification() in ControlSocket.cpp allocates memory on line 2847 and fails to free it.*

Line 2847 enthält folgendes Code: t_connop *op = new t_connop;

Tja, und nun? Was soll uns das sagen? Schau mal in der Dokumentation zu dem von Dir nicht näher spezifizierten Tool nach, wie so eine Meldung zu bewerten ist. Der Satz "The function..." ist sicherlich richtig. Aber das allein garantiert Dir noch kein Memory-Leak. Es könnte also ein flascher Alarm sein. Es könnte aber auch richtig sein, weil das Tool nirgends wo ein " delete p " gefunden hat, wobei p ein Ausdruck vom Typ t_connop* ist. Dann würde ich auch ein Speicherleck melden, allerdings nicht mit so einem blöden "The function..."-Text. Das ist nämlich so nichtssagend. Natürlich muss man nicht immer in derselben Funktion den Speicher freigeben, den man angefordert hat. Im Gegenteil. Der Freispeicher ist gerade dazu da, Objekte anzulegen, die eine längere Lebensdauer haben und nach verlassen einer Funktion nicht sofort wieder zerstört werden, so wie es bei automatischen Objekten der Fall wäre.

Gruß,
SP

david.dda

ich habe den Code von FileZillaServer mit 5 Source Code Analysatoren durchgelaufen. CppCheck und RATS - beide sind nicht kommerzielle Werkzeuge und finden nur offensichtliche Fehler, wie Gebrauch von gefährlichen C++ Funktionen (strcpy..) dann habe ich C++Test verwendet - dieses Tool läßt sich super konfigurieren und ist sehr übersichtlich, PC-Lint - da habe ich noch Schwierigkeiten bei der Konfig. und Fortify - von dem auch die Fehlermeldung stamm, Fortify ist einfach zu installieren, leicht in der Benutzung und hat viele Features (Summary, Recommendations, Diagram, Details...) für die Fehlermeldungen. Allerdigs konnte keines von diesen Tools den Memory leak über den wir die ganze Zeit sprechen finden.

Sebastian Pizer

david.dda schrieb:

Allerdigs konnte keines von diesen Tools den Memory leak [...] finden.

Warum wundert Dich das?

david.dda

also hälst Du nicht viel von diesen Tools?
Es sind statische Source Code Analysatoren, dynamische würden den leak vielleicht finden, aber die habe ich nicht ausprobiert. Ich weiß auch nicht wie die einzelnen Tools den Quellcode analysieren. In der Dokumentation zu diesen gibt es zwar einige Beispiele, diese sind aber eher ziemlich kurz und einfach, damit man so ungefähr weiß, wie ein memory leak.. im Quellcode gefunden wird:

Consider the example:
char *p, *q = 0;
p = malloc(10);
p = q; // Warning -- memory leak

We are able to issue a Warning because the return from malloc has an allocation flag that indicated that the returned value points to a freshly allocated region that is not going to be freed by itself.

david.dda schrieb:

Ich habe schon einige Bücher über C++ gelesen, aber weil es nur viel Theorie ohne Praxis war, versteht man vieles nicht so eindeutig.

Eigendlich ist es doch genau umgekehrt: Man kann die Praxis nur verstehen wenn man vorher die Theorie begriffen hat. Ansonsten läuft man Gefahr die Praxis nicht zu verstehen, sondern zu interpretieren, wodurch beliebig falsche Denkmodelle entstehen können, wie wir an diesem Beispiel zum Verhalten von Zeigern ja sehr schön beobachten konnten...

Technisch gesehen ist die Regel für Speicherverwaltung einfach: Jedes new muß durch ein passendes delete wieder freigegeben werden. Das wars auch schon. Memory-leaks entstehen immer dann, wenn diese Regel gebrochen wird.

david.dda schrieb:

ich habe den Code von FileZillaServer mit 5 Source Code Analysatoren durchgelaufen.

Code Analysator != Memory-leak detector. Du benutzt also die falschen Tools und kannst daher auch nix finden.

Das ist ungefähr so als würdest Du einen Schraubenzieher benutzen um einen Baum zu fällen... Und Dich dann wundern warum das nicht so richtig funktioniert...

loks schrieb:

david.dda schrieb:

Ich habe schon einige Bücher über C++ gelesen, aber weil es nur viel Theorie ohne Praxis war, versteht man vieles nicht so eindeutig.

Eigendlich ist es doch genau umgekehrt: Man kann die Praxis nur verstehen wenn man vorher die Theorie begriffen hat. Ansonsten läuft man Gefahr die Praxis nicht zu verstehen, sondern zu interpretieren, wodurch beliebig falsche Denkmodelle entstehen können, wie wir an diesem Beispiel zum Verhalten von Zeigern ja sehr schön beobachten konnten...

Dann wird es sicher besser, wenn er noch 10 Bücher liest, aber nix programmiert.

Artchi

loks schrieb:

david.dda schrieb:

Ich habe schon einige Bücher über C++ gelesen, aber weil es nur viel Theorie ohne Praxis war, versteht man vieles nicht so eindeutig.

Eigendlich ist es doch genau umgekehrt: Man kann die Praxis nur verstehen wenn man vorher die Theorie begriffen hat. Ansonsten läuft man Gefahr die Praxis nicht zu verstehen, sondern zu interpretieren, wodurch beliebig falsche Denkmodelle entstehen können, wie wir an diesem Beispiel zum Verhalten von Zeigern ja sehr schön beobachten konnten...

Manchmal wird einem aber die Theorie schlecht erläutert, und dann hilft nur noch Praxis. Ich habe in meinen C++-Anfängen (vorher intensiv Basic und Assembler) über Klasse und Polymorphie nichts verstanden. Weil das Buch einfach die Theorie und Beispiele schlecht herüber brachte. Erst durch intensive Praxis (intensive Arbeit an einem beruflichen C++ Projekt) ging mir wirklich von einem Moment auf den anderen ein Geistesblitz durch den Kopf. Danach konnte ich mein Wissen wieder durch Theorie weiter entwickeln.

drakon

Artchi schrieb:

Manchmal wird einem aber die Theorie schlecht erläutert, und dann hilft nur noch Praxis. Ich habe in meinen C++-Anfängen (vorher intensiv Basic und Assembler) über Klasse und Polymorphie nichts verstanden. Weil das Buch einfach die Theorie und Beispiele schlecht herüber brachte. Erst durch intensive Praxis (intensive Arbeit an einem beruflichen C++ Projekt) ging mir wirklich von einem Moment auf den anderen ein Geistesblitz durch den Kopf. Danach konnte ich mein Wissen wieder durch Theorie weiter entwickeln.

Dem stimme ich voll zu und ich denke, dass das auch sehr viele tun werden.
Das zeichnet sich imo dadurch aus, dass bei uns an der UNI Programmierung das wahrscheinlich Praxisorientierteste Fach von allen ist. Die Prüfung besteht hauptsächlich aus angewandten Beispielen und man muss da wirklich Code hin schreiben, anstatt irgendwelche Theoriefragen beantworten, die man einfach auswendig lernen könnte.

Speicherleck nur beim &quot;new&quot;?

Speicherleck nur beim "new"?