SFML: X11 Window Class does not free X11 properly!

[graphitemaster]

It's come to my concern that SFML causes tons of problems when running the simplest of things through valgrind. It seems as if SFML is not freeing X11 properly period, now I could be wrong about this I never looked at the source as I don't have it available, I do however have the headers, so I decided to take a peek it seems as if

 priv::WindowImpl* myWindow;         ///< Platform-specific implementation of window

which is a pointer, is never freed, now this could be handled somewhere else, and freed I'm not exactly sure. Valgrind reports tons of errors, regarding libX11 which can be seen is handled by libSFML. I went out of my way and made a mini test using libX11 and freeing everything myself and it passed valgrind -100%- however the same usage in SFML reported tons of errors, this is how I know SFML is not freeing X11 properly.

I have included a link below of the valgrind output on pastebin.com
I would like to see this fixed because I'm very retentive about memory
leaks and corruption, and when I'm testing my own code in valgrind I hate to
see my console, or a file that I have piped the valgrind output to be filled
with issues regarding SFML and X11, but rather the issue involved in my
code, so I can fix it.

VALGRIND: http://pastebin.com/hZ198eL4

[Laurent]

All the "leaks" seem to be related to the global OpenGL context, which is indeed never freed on purpose (destroying it would cause crashes, because it happens at global exit and doesn't mix well with some OpenGL drivers).

However this is done on purpose, and the memory is freed by the OS anyway. It's not like a constant leak making the memory usage grow over time.

The only real issue is that it pollutes your valgrind log.

[graphitemaster]

I'm not sure I follow, creating the exact same application without SFML, but instead using X11 directly and opening my own OpenGL context, and freeing it manually does not produce any errors with valgrind period.

How could freeing the context after you're finished using it cause a crash, and even if it did you only ever free the context to your window when you're finished with the program and want it to close, so a crash would not be an issue to anyone unless it was causing the program to close prematurely.

I'm sure for the most part, there should be a manual freeing function in SFML for the developers who do want to free the context regardless of what harm it could do, understanding the risks from things is something every developer knows.

Yes it does severely cause some undesired talk back in valgrind, and the longer the program is run the more back talk it generates, THIS SHOULD NOT HAPPEN.
It makes it incredibly hard to do proper debugging - which I understand most people using SFML don't do because SFML is such a high level API-

I'm having second thoughts on using SDL, or writing my own implementation for each operating system and platform for window context handling, Unlike most people here I'm not creating 2D side scrolling plat-formers, and my project that utilizes SFML is not a hobby project, rather a serious project that I have devoted most of my time to.

Please find a way to fix the issue, I beg you, valgrind begs you. Or else I'm going to half to find an alternative solution for my 3D engine.

[Laurent]

Well, this crash at global exit was so hard to solve that I finally gave up and changed it to a memory leak as a workaround. If you want to see by yourself, you just have to change line 82 in src/SFML/Window/Context.cpp to this:

Code: [Select]

static Context GlobalContext;
No more memory leak, but a crash. If I remember well, it happens when you use dynamic libraries and the default font of the graphics module.

You can also modify SFML to add your manual cleaning function.

I have a similar bug in SFML 2 that I'm trying to solve, but it's even more tricky, it happens only with ATI and Intel drivers.

[Silvah]

The only real issue is that it pollutes your valgrind log.

Unless the behavior is different on Windows, this sentence is not true. FAQ says the library works fine on Win98. This OS doesn't always free resources at exit, possibly resulting in memory (and probably not only memory) leak. If you want to be portable, you shouldn't rely on OS doing anything for you automatically, because some simply won't.

EDIT: rephrased to make it clearer.

[Laurent]

Unless the behavior is different on Windows, this sentence is not true. FAQ says the library works on Win98, which don't always free resources at exit, possibly resulting in memory (and probably not only memory) leak.

Really? Can you show me this FAQ?

If you want to be portable, you shouldn't rely on OS doing anything for you automatically.

I'm not relying on anything, just trying to make this crappy bug less noticeable for users. If you have a clean fix, I'd be glad to apply it.

[Silvah]

Really? Can you show me this FAQ?

Here you are. I think you misunderstood me (my bad, that wasn't as clear as it should be), that "FAQ says..." part lasts only to the comma

[Laurent]

Here you are. I think you misunderstood me (my bad, that wasn't as clear as it should be), that "FAQ says..." part lasts only to the comma

Ah, ok
My question was about the second part

Win98, which don't always free resources at exit

Do you have a source for that?

[graphitemaster]

don't know if it's the problem but I think I have found a bug

http://sfml.svn.sourceforge.net/viewvc/sfml/branches/sfml2/src/SFML/Window/Linux/GlxContext.cpp?revision=1570&view=markup

line 104-105
the display should be flushed before it's destroyed?

also
http://sfml.svn.sourceforge.net/viewvc/sfml/branches/sfml2/src/SFML/Window/Linux/GlxContext.cpp?revision=1570&view=markup

line 234
GLXFBConfig* configs = glXChooseFBConfig(myDisplay, DefaultScreen(myDisplay), NULL, &nbConfigs);

thats in a while loop, and never freed, thats just a mem leak unless I'm not seing something.

[Silvah]

Do you have a source for that?

Alas, I don't. I just vaguely remember that Win9x stores resources in some kind of memory pool (one for all programs), and if it ran out of space, the system crashed due to an alleged lack of free memory. Because some programs didn't free resources, and operating system didn't do that automatically, either,  the OS as a whole was utterly unstable.

[Laurent]

line 104-105
the display should be flushed before it's destroyed?

I don't remember if this line is really necessary. But it can't do harm, anyway.

line 234
GLXFBConfig* configs = glXChooseFBConfig(myDisplay, DefaultScreen(myDisplay), NULL, &nbConfigs);

Yup, this one was leaking, I fixed it. Thanks.

Alas, I don't. I just vaguely remember that Win9x stores resources in some kind of memory pool (one for all programs), and if it ran out of space, the system crashed due to an alleged lack of free memory. Because some programs didn't free resources, and operating system didn't do that automatically, either, the OS as a whole was utterly unstable.

Okay. I'll remember that.

[graphitemaster]

XVisualInfo* bestVisual = NULL;  line 184

http://sfml.svn.sourceforge.net/viewvc/sfml/branches/sfml2/src/SFML/Window/Linux/GlxContext.cpp?revision=1579&view=markup

not freed!?

[Laurent]

No, this is just a pointer to an element of the visuals array, and this one is freed at the end of the function.

I guess there's no more memory leak, otherwise it would appear in the valgrind log. The GLXFBConfig stuff was not detected because the corresponding code was not executed (OpenGL 3 specific).

[graphitemaster]

in that same while loop, you need a
delete [] name; that's also not freeded

const GLubyte* name = reinterpret_cast<const GLubyte*>("glXCreateContextAttribsARB");

line 229,

http://sfml.svn.sourceforge.net/viewvc/sfml/branches/sfml2/src/SFML/Window/Linux/GlxContext.cpp?revision=1579&view=markup

[Laurent]

yes in that same while loop, you need a
delete [] name; that's also not freeded

const GLubyte* name = reinterpret_cast<const GLubyte*>("glXCreateContextAttribsARB");

line 229,

http://sfml.svn.sourceforge.net/viewvc/sfml/branches/sfml2/src/SFML/Window/Linux/GlxContext.cpp?revision=1579&view=markup

String literals are not allocated dynamically, they don't need to be freed.

Maybe you should investigate a little bit more about the "leaks" that you report