[C++]Memory leak, maar kan hem niet vinden

[DDan]

Heb in bc++6 een programmaatje gemaakt dat wat html code genereert (een lijst van subdirectories en bestanden), dit gebeurd dmv een recursieve functie die steeds na een bepaalde tijd gestart wordt en afloopt zodra hij alle subdirs en bestanden van een opgegeven directory doorlopen heeft.

Programma structuur bestaat uit gewoon een hoofdunit + het form, en twee zelf gemaakte classes die een (gelinkte) lijst moeten voorstellen van de gevonden subdirs binnen een bepaalde directory, en daar zit volgens mij dan ook *ergens* de fout, alleen geef ik, zover ik kan zien, in die classes netjes alle geheugen vrij. Daarom ook de vraag of iemand anders daar eens een keer heen wil kijken.

Nu heb ik via de search een programma gevonden dat memproof heet, hiermee heb ik me eigen programma eens "getest" maar hieruit kan ik niet meer afleiden dan dat het taakbeheer me al kon vertellen: het blijft steeds meer geheugen toewijzen (zag alleen hoe gigantisch veel geheugen werd toegewezen via *een paar duizend * pointers). Aan de hoeveelheid geheugen te zien die het programma gebruikt ben ik echt iets heel vreemds aan het doen (bij start al zo'n 6MB en na paar x die recursieve functie aanroepen al gauw zo'n 14MB waarna hij ook komt met een error). bvd.

De code (rar-filetje)

[Klippy]

't is nogal laat en wel erg gemakkelijk om hele code hier te posten, is niet makkelijk uit te vinden voor iemand die het programa niet gemaakt heeft wat het programma precies doet, laat staan waar de fout zit.

Probeer eens zelf te kijken in welke functie de fout zit door te debuggen. Debuggen gaat trouwens ook niet makkelijk zo omdat je steeds een minuut moet wachten voordat je kan beginnen...

Voeg ook eens wat commentaar toe aan de code is altijd handig

Ik heb code bestudeerd, maar ik zie idd ook zo snel geen fout, maar wel paar dingen die ik niet helemaal snap, waar je de output file leeg maakt bijvoorbeeld, zie alleen addline staan dus lijkt alsof die code steeds eraan geplakt wordt ipv overschreven...
Verder zie ik ook niet veel, maar heb eigenlijk ook nog geen groot lek kunnen zien. Heb 't niet zo lang laten lopen..

Misschien dat ik morgen nog wel ff kijk als 't topic dan nog open is, ben ik wat helderder

[ Voor 7% gewijzigd door Klippy op 16-07-2003 04:57 ]

[Verwijderd]

Zet codeguard is aan. Die geeft, als je programma memleaks heeft, welk(e) object(en) niet gedelete worden.

[DDan]

LiquidSilver schreef op 16 July 2003 @ 04:54:
...'t is nogal laat en wel erg gemakkelijk om hele code hier te posten, is niet makkelijk uit te vinden voor iemand die het programa niet gemaakt heeft wat het programma precies doet, laat staan waar de fout zit...

...Voeg ook eens wat commentaar toe aan de code is altijd handig ...

......Verder zie ik ook niet veel, maar heb eigenlijk ook nog geen groot lek kunnen zien. Heb 't niet zo lang laten lopen...

Wat het programma doet heb ik een beetje proberen uit te leggen in mijn eerste post, verder heb je natuurlijk helemaal gelijk

Probeer eens zelf te kijken in welke functie de fout zit door te debuggen. Debuggen gaat trouwens ook niet makkelijk zo omdat je steeds een minuut moet wachten voordat je kan beginnen...

Het "met F7 er doorheen lopen" heb ik al een behoorlijk aantal keer gedaan en zo dacht ik ook een aantal keer het probleem gevonden te hebben (zoals bijvoorbeeld de destructors die niet werden aangeroepen van die twee gemaakte classes) maar dat bleek het dan toch weer niet te zijn.

Ik heb code bestudeerd, maar ik zie idd ook zo snel geen fout, maar wel paar dingen die ik niet helemaal snap, waar je de output file leeg maakt bijvoorbeeld, zie alleen addline staan dus lijkt alsof die code steeds eraan geplakt wordt ipv overschreven...

die files waar die addline op toegepast wordt, worden geopend met 'fopen(filenaam,"w");' oftewel ze worden geopend om te worden overschreven.

Verwijderd schreef op 16 juli 2003 @ 11:31:
Zet codeguard is aan. Die geeft, als je programma memleaks heeft, welk(e) object(en) niet gedelete worden.

Thnx, kende dat codeguard nog niet heb er idd al wat kleine foutjes mee kunnen vinden

[.oisyn]

En anders is er nog Rational Purify

[Verwijderd]

Als je een linked list maakt, moet je wel even in de destructor van de dirlist alle dirnodes netjes wegpoetsen. Jouw destructor is leeg.... Nu wordt alleen de startpointer weggegooid...

Overigens, Borland heeft hele mooie containers voor dit soort grappen, zoals TList en TStringList. Ook in de STL zitten handige containers. Die zorgen ervoor dat je dit soort gedonder niet meer hebt....

Het gebrek aan commentaar is trouwens inderdaad vervelend. Ik heb nog steeds geen idee wat het proggie moet doen

[DDan]

code:

//in de header file van de dirlist
~DIRLIST() {for (int i = 0; i < sz; i++) (*this)--;} 
//roept voor elke dirnode operator-- aan

//in de cpp file van dirlist
char* DIRLIST::operator--(int){
  char *temp = first->getDirName();
  DIRNODE *dn;

  sz--;
  dn = first;
  first = first->getNext();
  delete dn;
  return temp;
}

Dit is wat ik toegevoegd heb, getNext() retouneert gewoon een pointer naar een dirnode, en zodra ik delete zeg voert hij ook netjes de bijbehorende destructor uit (voor zover ik kan zien dan), alleen het memleak blijft

Denk dat ik idd eens ga kijken naar TStringList en gewoon die classes uit het project donder

edit:
wat me nu opvalt is, dat ik (in de operator--) een char* retouneer naar geheugen wat officieel vrijgegeven zou moeten zijn na het uitvoeren van de destructor van een dirnode, dus daar klopt ook nogal weer iets niet aan...

[edit2]
Even omgezet naar TStringList, maar memleak blijft, ondanks dat codeguard zich nu ook helemaal niet meer meldt (en ja de TStringList wordt netjes vrijgegeven met een delete, zelfs nog even een klein tellertje op gezet op het aantal new/delete's om er zeker van te zijn) *gaat rest van hoofdprogramma nog maar eens doorspitten*

[ Voor 33% gewijzigd door DDan op 16-07-2003 15:48 ]

[.oisyn]

C++:

~DIRLIST()
{
    for (int i = 0; i < sz; i++)
        (*this)--;
}

dat klopt natuurlijk ook niet he
Bij elke operator -- () aanroep zal sz met 1 worden verlaagd. Dus behalve dat i oploopt, loopt sz ook nog eens af, met als gevolg dat je maar de helft vrijgeeft

dit lijkt me dan beter:

C++:

~DIRLIST ()
{
    while (sz > 0)
        (*this)--;
}

[DDan]

.oisyn schreef op 16 juli 2003 @ 16:14:
dat klopt natuurlijk ook niet he
...Bij elke operator -- () aanroep zal sz met 1 worden verlaagd. Dus behalve dat i oploopt, loopt sz ook nog eens af, met als gevolg dat je maar de helft vrijgeeft...

Heb het even aangepast en nog eens met classes getest, maar nix, nog steeds blijft het gebruikte hoeveelheid geheugen toenemen. Tot nu toe verder nog niets kunnen ontdekken in het hoofdprogramma, en daar zal het toch ergens moeten zitten aangezien het probleem ook met het gebruik van TStringList nog steeds niet weg is...

[Verwijderd]

Nou ja,

Ik wil niet erg vervelend doen maar memory leaks kan je zelf localiseren. Dit kan misschien wel een goed idee zijn!

Over het algemeen weet je heel goed wanneer je iets alocate en dan weer de-alocate in je geheugen. Als je hierom eens wat break-points neer zet kan checken (debug tool/ taskmanager) of er memory leak optreed, met het gevolg dat je een uitspraak kan doen waar de memory leak optreed.

Als je weet waar de memory leak optreed en je er niet uit komt hoe je het kan oplossen. dan kan je het volgende doen:

Zet dat geisoleerde code fragment op het forum en geef hier eventueel de header-files mee van de gebruikte Classes/struct. Op deze manier krijg jij wat antwoorden waaraan jij iets hebt. en voor ons is het allemaal nog te overzien, lekker efficient!

Ik moet wel zeggen dat het wat meer inspanning verwacht van joun zijde!

[DDan]

@Marik: Die inspanning is er, heb hier de nodige dagen al ingestoken om te kunnen ontdekken wat er nu mis is (en nee die opmerking in dat laatste regeltje zint me absoluut niet)... Maar kga wat je zegt iig eens proberen, bedankt voor de tip

[Verwijderd]

Het enige dat ik verder nog kan bedenken is dat die recursie actie van jou niet helemaal lekker werkt. Je roept de functie GenerateHTML aan vanuit zichzelf. Ik vindt het al geen schoonheidsprijs waard, maar je kunt hierdoor ook vrij snel door je stackpointers heen raken. Ook je geheugenbeheer raakt danig in de war denk ik. Ik zou dit anders opgelost hebben....

[DDan]

Verwijderd schreef op 17 July 2003 @ 15:58:
Het enige dat ik verder nog kan bedenken is dat die recursie actie van jou niet helemaal lekker werkt. Je roept de functie GenerateHTML aan vanuit zichzelf. Ik vindt het al geen schoonheidsprijs waard, maar je kunt hierdoor ook vrij snel door je stackpointers heen raken. Ook je geheugenbeheer raakt danig in de war denk ik. Ik zou dit anders opgelost hebben....

Vond het eigelijk wel een leuke oplossing voor dit probleem, op deze manier geeft hij namelijk alle directories en bestanden weer op de volgorde waarop ik ze bedoeld had. Voor zover ik weet kon ik dit niet bereiken met een wat simpeler while/for-lus constructie. Maar goed, er zullen zeker andere (nettere) oplossingen zijn edit: Overigens zou die recursie geen enkel probleem moeten zijn, want in een oudere versie van dit programma had ik geen problemen hiermee (daar zat trouwens ook geen memleak in)

functies die gevonden zijn op de manier van marik die *soms* tot 200kB geheugen oppakken en het niet meer vrijgeven: (de andere functie doet hetzelfde maar dan met een memobox als parameter)

C++:

void __fastcall TForm1::writeData(FILE* ofp, TEdit* textBox){
  char* line = new char[(textBox->Text).Length() + 2];

  for (int ix = 0; ix < (textBox->Text).Length(); ix++)
    line[ix] = textBox->Text[ix + 1];
  line[(textBox->Text).Length()] = '\n';
  line[(textBox->Text).Length() + 1] = '\0';
  fprintf(ofp, line);
  delete []line;
}

lijkt er dus op dat die line niet helemaal correct wordt vrijgegeven. Het geheugen gebruik groeit ook pas in de for-lus en niet direct na het gebruik van de new (wat me eigelijk de bedoeling lijkt te zijn?)

[ Voor 10% gewijzigd door DDan op 17-07-2003 16:24 ]

[Glimi]

DDan schreef op 17 July 2003 @ 16:20:

C++:

(...)
  delete []line;
}

Als dit er zo letterlijk staat, zie ik eigenlijk niet in waarom het zou compileren. Maar maybe dat het de fout is dat je de operator delete aanroept ipv delete[], let op de spatie
_{Of zit ik weer te suffen vandaag?}

[.oisyn]

Glimi, je lijkt whoami wel . De [] is geen deel van het keyword, maar gewoon twee losse chars. Het aantal spaties tussen de delete en de [], of het aantal tussen de [ en de ] boeit dus totaal niet.

Jij hebt toch ook wel enige compilerbouw kennis of heb ik me vergist?

[ Voor 17% gewijzigd door .oisyn op 17-07-2003 20:26 ]

[MSalters]

DDan schreef op 17 juli 2003 @ 16:20:

C++:

void __fastcall TForm1::writeData(FILE* ofp, TEdit* textBox){
  char* line = new char[(textBox->Text).Length() + 2];

  for (int ix = 0; ix < (textBox->Text).Length(); ix++)
    line[ix] = textBox->Text[ix + 1];
  line[(textBox->Text).Length()] = '\n';
  line[(textBox->Text).Length() + 1] = '\0';
  fprintf(ofp, line);
  delete []line;
}

Dit gaat heel erg fout als line een % teken bevat. Als er bv %s in staat, dan zal fprintf( ofp, line ) een char* verwachten. Omdat die er niet staat, corrupt je je stack. Oplossing:

C:

fprintf( ofp, "%s", line );

[.oisyn]

MSalters schreef op 17 July 2003 @ 21:38:
Omdat die er niet staat, corrupt je je stack. [/code]

nou ja de stack zal niet corrupten. fprintf is cdecl, dus de caller ruimt de stack op.

[MSalters]

Duh, je hebt natuurlijk gelijk. fprintf weet tenslotte niet hoeveel argumenten er opgeruimd moeten worden, dus dat gaat maar een beetje fout.

Het enige wat er wel erg fout kan gaan is een (POSIX) %n, die schrijft naar een int*. Aangezien die niet meegegeven wordt worden er 4 random bytes van de stack als adres misbruikt en worden dus 4 willekeurige bytes overschreven. Als ik zo kijk wat er op de stack staat is dat char* line, dus je hebt een kans dat de eerste 4 chars van line overschreven worden.

_{'t is wel errug obscuur}

[DDan]

Probleem opgelost, schuldige code was deze (de for-lus):

C++:

void __fastcall TForm1::writeMemoData(FILE* ofp, TMemo* memoBox){
    char* line = new char[strlen(memoBox->Lines->GetText()) + 1];

    for (unsigned ix = 0; ix < strlen(memoBox->Lines->GetText()); ix++)
      line[ix] = memoBox->Lines->GetText()[ix];
    line[strlen(memoBox->Lines->GetText())] = '\0';
    fprintf(ofp, line);
    delete []line;
}

Deze functie en de vorige gelijksoortige functie (writeData) eruit gemikt en vervangen door fprintf(), die wel werkt op de manier waarop MSalters aangaf voor strings (kreeg dat namelijk eerst niet werkend omdat hij zonder die extra parameter gewoon een char* verwacht en geen string). Weer wat geleerd Thnx voor jullie hulp

[Verwijderd]

edit:
never mind, het was al klaar

[ Voor 90% gewijzigd door Verwijderd op 18-07-2003 00:49 ]

[MSalters]

De wijze les: Gebruik geen formatted output functie (printf) als je niet wil formatten.

[LordLarry]

.oisyn schreef op 16 July 2003 @ 13:51:
En anders is er nog Rational Purify

Of http://www.automatedqa.com/products/aqtime.asp.

Die weet ook nog van de VCL af die BCB gebruikt en kan dus nog gemakkelijker en meer informatie verschaffen.

Maar de ingebouwde CodeGuard kan je vaak ook veel vertellen.

[ Voor 11% gewijzigd door LordLarry op 18-07-2003 14:57 ]