garrynewman / GWEN

Abandoned: GWEN - GUI Without Extravagant Nonsense.
MIT License
429 stars 102 forks source link

UnitTest memory leaks #16

Closed Cheaterdev closed 11 years ago

Cheaterdev commented 11 years ago

There are memory leaks somewhere in UnitTest. I see many "new" but not "delete".

qehgt commented 11 years ago

Just run some tests to check memory leaks. It looks like something wrong with TextBoxMultiline - if I remove creation of such controls from UnitTest (in UnitTest/TextBox.cpp and UnitTest/LabelMultiline.cpp), there are no memory leaks.

qehgt commented 11 years ago

@Cheaterdev: most de-allocations are handled by GWEN library itself.

sreich commented 11 years ago

run it under a memchecker and prove it then. i've run it under valgrind several times and it hasn't caught anything.

and yes, gwen is much like qt in the sense that it has a concept of parenting for the widgets (which is why you usually see = new somewidget(this)).

though it did have a lot of using of undefined values, which i've fixed in https://github.com/sreich/NEWGWEN

qehgt commented 11 years ago

@sreich https://gist.github.com/4657566 My modifications to use build-in memory checker: https://gist.github.com/4657602

Again, there are no memory leaks if TextBoxMultiline is not created

sreich commented 11 years ago

argh, this is the most worthless leak checker i've ever seen. use something good, like valgrind or dr. memory (the latter which i've heard is good, but haven't used it myself).

qehgt commented 11 years ago

@sreich, thanks for your advice, but you asked about a prove, so I did it. I know about valgrind or other debug tools, but the fastest way to check for memory leaks on Windows platform is to use build-in functionality of MS CRT library (no additional software is needed, and only 2 lines of code had to be added into source file).

sreich commented 11 years ago

yes, except the information it spewed out is completely worthless. it's not a bt, it's just a useless hexdump. doesn't help anything, and just says "durrrr, there's a memory leak somewhere".

qehgt commented 11 years ago

Yes, I totally agree. I am wondering why valgrind did not catch that leak. I'll run it on my Linux box later to check it.

qehgt commented 11 years ago

@sreich, I just have run it with valgrind, and got same result: https://gist.github.com/4660257

Diff: https://gist.github.com/4660269

Run: valgrind --leak-check=yes ./CrossPlatformSample_Dd

qehgt commented 11 years ago

Hm... and now https://github.com/garrynewman/GWEN/issues/19 looks like the same issue

sreich commented 11 years ago

this should fix it, haven't verified it though yet. feel free to apply the patch yourself, i committed it to NEWGWEN:

https://github.com/sreich/NEWGWEN/commit/b3367d86f7a948954039937ecd65b18f4ab01e02

XTracer commented 11 years ago

@sreich thanks, it's works!

qehgt commented 11 years ago

@sreich, @garrynewman I confirm, no more leaks PS: https://github.com/qehgt/GWEN/commit/6d646e9843ad9410898cc1a4d48091c2417c9772 - replace spaces with tabs in @sreich's commit