-
A minimal example:
sf::RenderWindow* ptr = new sf::RenderWindow(sf::VideoMode(800, 600, 32), "title");
ptr->setActive(false);
while (true)
{
std::thread([=]()
{
ptr->setActive(true);
ptr->setActive(false);
}
).join();
}
Memory keeps rising (from ~12MB to over 300MB). Requesting confirmations.
-
Please provide the exact SFML version (i.e. from where you got it) and how you've been checking the 'memory leak'. Then you should provide a complete example (with main() and inclusions) that doesn't have a memory leak by definition (you never call delete).
It might well be that the memory you see allocated is just made available from the OS to the application, without the application actually using it. OS memory allocation isn't just a simple allocation/release on request, it's way more complex and there's mostly no way to actually predict when what happens. Thus a correct memory leak detection software is needed, although they might also show memory leaks where non are.
-
...
My exact version appears to be "SFML-2.0-rc-102-g044eb85" and was downloaded from this website (the github link) on the 10th of November 2012. I check memory usage with the "System Monitor" provided by Linux Mint 13 x64.
#include <SFML/Graphics.hpp>
#include <thread>
int main()
{
sf::RenderWindow* ptr = new sf::RenderWindow(sf::VideoMode(800, 600, 32), "title");
ptr->setActive(false);
while (true)
{
std::thread([=]()
{
ptr->setActive(true);
ptr->setActive(false);
}
).join();
}
return 0;
}
-
I just downloaded the latest repositories (LaurentGomila-SFML-9fac5d7). Built both Release and Debug mode using CMake. The error persists.
-
It's not a leak. The memory will be freed when the window is destroyed. It rises because internally, because of some ugly magic, a new OpenGL context is created in each new thread. And since SFML is not notified when a thread dies, it can only release the associated context at the "end" (when no SFML resource exist anymore). Hopefully, in a real application you won't create drawing threads in a loop like that ;)
-
What about the following code that causes a segmentation fault? Without the thread, it does not throw a segfault. Why?
#include <SFML/Graphics.hpp>
#include <thread>
int main()
{
sf::RenderWindow* ptr;
while (true)
{
ptr = new sf::RenderWindow(sf::VideoMode(800, 600, 32), "title");
ptr->setActive(false);
std::thread([=]()
{
ptr->setActive(true);
ptr->setActive(false);
}
).join();
ptr->setActive(true);
ptr->close();
std::this_thread::sleep_for(std::chrono::seconds(1));
delete ptr;
}
return 0;
}
#0 0x7ffff1749239 ??() (/usr/lib/fglrx/dri/fglrx_dri.so:??)
#1 0x7ffff17b040e ??() (/usr/lib/fglrx/dri/fglrx_dri.so:??)
#2 0x7ffff18878c2 ??() (/usr/lib/fglrx/dri/fglrx_dri.so:??)
#3 0x7ffff17a0384 ??() (/usr/lib/fglrx/dri/fglrx_dri.so:??)
#4 0x7ffff2444295 ??() (/usr/lib/fglrx/dri/fglrx_dri.so:??)
#5 0x7ffff243fa73 ??() (/usr/lib/fglrx/dri/fglrx_dri.so:??)
#6 0x7ffff2440cb6 ??() (/usr/lib/fglrx/dri/fglrx_dri.so:??)
#7 0x7ffff244fa68 ??() (/usr/lib/fglrx/dri/fglrx_dri.so:??)
#8 0x7ffff244ff86 ??() (/usr/lib/fglrx/dri/fglrx_dri.so:??)
#9 0x7ffff3078e83 ??() (/usr/lib/fglrx/dri/fglrx_dri.so:??)
#10 0x7ffff2ee8dc9 ??() (/usr/lib/fglrx/dri/fglrx_dri.so:??)
#11 0x7ffff1743112 ??() (/usr/lib/fglrx/dri/fglrx_dri.so:??)
#12 ( 0x00007fffffffd6c0 in ??() (??:??)
#13 0x7ffff30e0891 ??() (/usr/lib/fglrx/dri/fglrx_dri.so:??)
#14 ( 0x0000000000000026 in ??() (??:??)
#15 0x7ffff7de992d ??() (/lib64/ld-linux-x86-64.so.2:??)
-
Could you recompile in debug and try to get a more detailed call stack?
-
I did, identical call stack.
-
Did you compile SFML in debug mode?
-
Most definitely, I even rebuilt them right now. I link to the -d.so files, which were created by the created project when entering Debug in CMake.
However, I think this may be an ATI / AMD driver problem:
https://bbs.archlinux.org/viewtopic.php?pid=401468
Edit:
Installing fglrx updates (using Synaptic)...
-
However, I think this may be an ATI / AMD driver problem:
https://bbs.archlinux.org/viewtopic.php?pid=401468
Have you tried the proposed fix?
-
I haven't tried it in the exact form because I do not have Arch Linux, but I downloaded the latest drivers and even restarted. It does not help my issue.
Edit: Seems to be a common problem when searching about fglrx on the internet (gamers, programmers mostly). Very annoying tho. I'll see if I can edit my design a bit so the drawing thread can just transfer control to a different location instead of being re-created.
Edit2: Is there no way to force some kind of cleanup? I'm gonna try to edit the some sfml sources...
-
Is there no way to force some kind of cleanup?
There is. Play with the private functions in GlContext.cpp and make public what you need.
-
There is. Play with the private functions in GlContext.cpp and make public what you need.
I looked at it and it's an abstract base class with only a one private function, and two protected functions.
protected :
GlContext();
static int evaluateFormat(unsigned int bitsPerPixel, const ContextSettings& settings, int colorBits, int depthBits, int stencilBits, int antialiasing);
ContextSettings m_settings; ///< Creation settings of the context
private:
void initialize();
On Linux we implement GlxContext, so I opened the GlxContext .hpp and .cpp. I'm focussing on the destructor, since I'm using this testing code:
#ifdef DEBUG
#include <iostream>
#endif
#include <SFML/Graphics.hpp>
#include <thread>
int main()
{
// The drawer thread, active during the entire program's lifetime, constant drawing ////////////
std::thread([&]()
{
sf::RenderWindow wnd(sf::VideoMode(300, 200, 32), "Title");
wnd.setFramerateLimit(1);
while (true)
{
wnd.clear();
wnd.display();
}
}
).detach();
//////////////////////////////////////////////////////////////////////////////////////////////
// Simulation of switching states ////////////////////////////////////////////////////////////
while (true)
{
std::thread([&]()
{
sf::RenderTexture rt;
std::this_thread::sleep_for(std::chrono::seconds(1));
} // rt should be destroyed here, but its effect on the memory remains.
).join();
}
//////////////////////////////////////////////////////////////////////////////////////////////
return 0;
}
And whenever a window is active, the RenderTexture destructor is NOT properly called! Try to comment out the window thread, it'll not leak. With the window, it does leak! If we un-thread the state-switcher, it also does not leak. So I decided to take a closer look at the destructor:
GlxContext::~GlxContext()
{
// Destroy the context
if (m_context)
{
if (glXGetCurrentContext() == m_context)
glXMakeCurrent(m_display, None, NULL);
glXDestroyContext(m_display, m_context);
}
// Destroy the window if we own it
if (m_window && m_ownsWindow)
{
XDestroyWindow(m_display, m_window);
XFlush(m_display);
}
// Close the connection with the X server
CloseDisplay(m_display);
}
Seems alright, we then have code from the base class's virtual destructor:
GlContext::~GlContext()
{
// Deactivate the context before killing it, unless we're inside Cleanup()
if (sharedContext)
setActive(false);
}
An old foe? I showed that setActive causes memory expansion. Now even when the object is destroyed, memory is expanded. Does it have something to do with this? What about the functions in the GlxContext destructor? XFlush is ignored when the window is not owned? Am I even on the right tracks here?
Anyway, I removed the contents of ~GlContext(), however, I now get an error when rebuilding my testing scenario about an error in WindowStyle.hpp, line 40. Wat do?
-
It's not a leak. The memory will be freed when the window is destroyed.
Actually if you ask me, the program doesn't leak, but within the program there is a leak because memory isn't being freed when a resource is not used anymore. It's like a server daemon leaking one byte every minute, but the daemon developer saying "our daemon doesn't leak, the allocator frees everything on exit". I wouldn't want to run that for too long ;).
It rises because internally, because of some ugly magic, a new OpenGL context is created in each new thread.
Yes the context management is really black magic :P. I already tried going through it many times but gave up after a few hours ^^.
And since SFML is not notified when a thread dies
What? Since when can you not be notified of a thread terminating? There are ways to do this...
-
I looked at it and it's an abstract base class with only a one private function, and two protected functions.
I was talking about the stuff hidden inside GlResource.cpp. And "make public" meant "write the functions that you need, using this stuff", not just "change a 'private' to 'public'" ;)
Don't bother investigating in the destructors, there's no mistake in the code. The "leak" happens because each new thread creates an internal/hidden context, but cannot destroy it when the thread ends. So contexts accumulate until all SFML resources have been destroyed (there's a global reference counter which triggers the globalCleanup() function).
Yes the context management is really black magic . I already tried going through it many times but gave up after a few hours ^^.
I don't like this part of the code, but at least it works ;D
What? Since when can you not be notified of a thread terminating? There are ways to do this...
How? I have no knowledge about the thread itself, it's created by the user. I only "know" that I'm in a new thread because I use a thread-local variable.
-
(there's a global reference counter which triggers the globalCleanup() function).
It was found. Also this:
sf::priv::GlContext* getInternalContext()
{
if (!hasInternalContext())
{
internalContext = sf::priv::GlContext::create();
sf::Lock lock(internalContextsMutex);
internalContexts.insert(internalContext);
}
return internalContext;
}
You seem to delete the entire set when there are 0 contexts. I'm busy creating a function that deletes a single context + remove the pointer from the set with the only parameter being the Texture. Tried several times now, kinda like walking into a maze...
-
I'm really sorry, I don't have time to write and test a solution for you.
I think you should:
- create a public function named "releaseThreadContext"
- inside this function, you remove internalContext from the internalContexts array and then you delete it
But this will work only if you nothing after executing this function (ie. not calling setActive(false) or destroying a render-window/render-texture).
-
I'm really sorry, I don't have time to write and test a solution for you.
I think you should:
- create a public function named "releaseThreadContext"
- inside this function, you remove internalContext from the internalContexts array and then you delete it
But this will work only if you nothing after executing this function (ie. not calling setActive(false) or destroying a render-window/render-texture).
#ifdef LINUX
#include <X11/Xlib.h>
#endif
#include <SFML/Graphics.hpp>
#include <thread>
int main()
{
#ifdef LINUX
XInitThreads(); // Required on Linux to handle multiple threads with windowing.
#endif
// The drawer thread, active during the entire program's lifetime, constant drawing ////////////
std::thread([&]()
{
sf::RenderWindow wnd(sf::VideoMode(300, 200, 32), "Title");
wnd.setFramerateLimit(1);
while (true)
{
wnd.clear();
wnd.display();
}
}
).detach();
//////////////////////////////////////////////////////////////////////////////////////////////
// Simulation of switching states ////////////////////////////////////////////////////////////
while (true)
{
std::thread([&]()
{
sf::RenderTexture *rt = new sf::RenderTexture;
delete rt;
sf::GlResource::releaseThreadContext();
std::this_thread::sleep_for(std::chrono::seconds(1));
} // rt should be destroyed here, but its effect on the memory remains.
).join();
}
//////////////////////////////////////////////////////////////////////////////////////////////
return 0;
}
I gave sf::GlResource the following public static member function:
static void releaseThreadContext();
---
void GlResource::releaseThreadContext()
{
priv::GlContext::releaseThreadContext();
}
So I thus gave sf::priv::GlContext the following public static member function:
static void releaseThreadContext();
---
void GlContext::releaseThreadContext()
{
sf::Lock lock(internalContextsMutex);
internalContexts.erase(internalContext);
delete internalContext;
}
However, no resources are freed in my testing scenario. I wonder what I am doing horribly wrong.
EDIT:
I replaced the
void GlContext::releaseThreadContext();
internals with the following:
sf::Lock lock(internalContextsMutex);
for (std::set<GlContext*>::iterator it = internalContexts.begin(); it != internalContexts.end(); ++it)
delete *it;
internalContexts.clear();
And it seems to work even when I have already-existing sprites being drawn, among other things. I'll now test it in my program to see if it works just as well there.
EDIT2: TOTALLY AWESOME! IT WORKS! Aaaah finally, after all these hours (so happy). I basically call sf::GlResource::releaseThreadContext(); after my drawing thread finishes, so another drawing thread can start drawing. This works perfectly, no segfaults, no error messages from OpenGL, nothing. Absolutely wonderful. Magnificent-> *good feels* :). But now it would need to be tested thoroughly. I really hope that this is something that stays eternally stable. It should also be a nice feature to add officially to make SFML more thread-friendly... Aaah, now I can rest :D
-
How? I have no knowledge about the thread itself, it's created by the user. I only "know" that I'm in a new thread because I use a thread-local variable.
There are only 2 ways a thread can terminate, either the thread function ends normally in which case execution will return to ThreadImpl::entryPoint(void* userData), or the user chooses to terminate the thread with terminate() in which case ThreadImpl::terminate() will be called. In both cases SFML is aware that the thread ends, you just don't make note of it and carry on with the application execution.
I know you don't want to make System depend on Window, so in this case you would just have to create a generic API that can notify observers that a thread has ended and it is up to them what they want to do. This would also be useful when attaching dynamic memory to TLS objects since you can be sure that they will be deallocated when the thread ends. Right now if you terminate a thread, it will cause a leak because the deallocator inside the thread function isn't called.
And regarding std::set<sf::priv::GlContext*> internalContexts;:
You already store all contexts inside this mutex-guarded set. You just don't have any association between which contexts belong to which thread and whether or not those threads are still alive. If you stored the association inside a std::map then you could easily get rid of dead contexts even during the runtime of the application. You just have to be notified of a thread terminating as per above and then remove the associated context respectively.
You probably don't want to focus on making the threading API more versatile right now, but advanced thread usage is impossible which such a limited API. I don't have the same problem as 7krs right now because as you know I avoid threading where possible especially for draw related operations. But some day someone will come with a really good use case and providing him with such a weak API will really deter the more "serious" library users.
But now it would need to be tested thoroughly. I really hope that this is something that stays eternally stable. It should also be a nice feature to add officially to make SFML more thread-friendly...
Getting rid of all internal GLContexts every time a thread ends isn't a clean solution. If it weren't for context sharing (which I despise with passion) your OpenGL resources would also disappear every time a thread ends. Not only that, GLContext creation is relatively expensive and doing it more times than needed will significantly impact the performance of your application. If you are content with this solution for your own needs (where I assume memory consumption is more important than application execution speed) then this is an acceptable solution. However most users don't share this requirement and your code probably will not be used in SFML as it is not a clean solution.
-
IT WORKS
You delete all the internal contexts, even those which might be in use in other threads.
There are only 2 ways a thread can terminate, either the thread function ends normally in which case execution will return to ThreadImpl::entryPoint(void* userData), or the user chooses to terminate the thread with terminate() in which case ThreadImpl::terminate() will be called. In both cases SFML is aware that the thread ends, you just don't make note of it and carry on with the application execution.
I was talking about threads in general, not sf::Thread. Look, he's using std::thread in his example.
-
You delete all the internal contexts, even those which might be in use in other threads.
I realize this, for trying to delete the thread-local context does not seem to work. The weird thing that surprised me is that all the other drawers stayed valid, they drew their rendertextures and updated them succesfully. After this the screen_drawer rendered the sprites of these textures succesfully.
Getting rid of all internal GLContexts every time a thread ends isn't a clean solution. If it weren't for context sharing (which I despise with passion) your OpenGL resources would also disappear every time a thread ends. Not only that, GLContext creation is relatively expensive and doing it more times than needed will significantly impact the performance of your application. If you are content with this solution for your own needs (where I assume memory consumption is more important than application execution speed) then this is an acceptable solution. However most users don't share this requirement and your code probably will not be used in SFML as it is not a clean solution.
I am not constantly switching states if that is what you think, I'm aiming for different states that last a long time, and I do not want to create many shared drawables and a single drawer thread. So the GlContexts are basically created upon entering a new major state in my program. So deleting all contexts is useful in my situation, but I would agree (if you imply); that it is more useful to let every thread kill its own context.
-
I was talking about threads in general, not sf::Thread. Look, he's using std::thread in his example.
Well in that case I don't think it is even safe, as Microsoft documentation states that you need to create and manage threads using one of their library functions to be able to use TLS without unintended side effects. I don't know how std::thread is implemented but I would not try to and mix and match if I don't need to.
The weird thing that surprised me is that all the other drawers stayed valid, they drew their rendertextures and updated them succesfully. After this the screen_drawer rendered the sprites of these textures succesfully.
This is because Laurent inserted ensureGLContext(); checks everywhere and new GLContexts are created every time they are needed and no valid one exists.
-
Well in that case I don't think it is even safe, as Microsoft documentation states that you need to create and manage threads using one of their library functions to be able to use TLS without unintended side effects. I don't know how std::thread is implemented but I would not try to and mix and match if I don't need to.
How could you create a thread on Windows without using one of the functions provided by the Windows API?
-
What my point was is that you don't have any information about how std::thread is implemented. It might be implemented differently in different runtimes. I know that ultimately they all need to use some kind of kernel functions to create a thread, but if e.g. std::thread was implemented using CreateThread as opposed to _beginthreadex then more care has to be taken that strange things don't start to happen when using SFML constructs inside threads because the assumption was that SFML has its own threading facility that is based around _beginthreadex and thus it is safe to use library functions that are permitted only when using that method of thread creation.
-
This is because Laurent inserted ensureGLContext(); checks everywhere and new GLContexts are created every time they are needed and no valid one exists.
I didn't know that function was called upon each draw call, explains a lot, thanks.
I figure that this solves my issue quite well, so I'm done here.
Thanks for the hints Laurent!
-
Decided that a lib should at adapt to me, not the other way around. So I started messing around...
I know this is an old thread, but I had to bump it because I've found what I believe to be a very good fix that may help anyone with the same problem. (We all want improvement right? :-X)
in sf::priv::GlContext we define the static method "clean":
// Notifies SFML that a thread is ending, and that the context needs to be deleted.
void GlContext::clean()
{
sf::Lock lock(internalContextsMutex);
GlContext *ptr = getInternalContext();
delete ptr;
internalContexts.erase(ptr);
// std::cout << "The following contexts are present:\n";
// for (std::set<GlContext*>::iterator it = internalContexts.begin(); it != internalContexts.end(); ++it)
// {
// std::cout << "\t" << *it << std::endl;
// }
}
The std::couts are for checking if there are garbage contexts left in the internalContexts set.
In GlResource we define a static member function that calls priv::GlContext::clean();.
When sf::GlResource::releaseThreadResource(); is called at the end of a thread, the resources are cleared.
Here's my test code:
#include <SFML/Graphics.hpp>
#include <thread>
#include <cstdio>
#include <X11/Xlib.h>
int main()
{
XInitThreads();
sf::RenderWindow wnd(sf::VideoMode(800, 600, 32), "Title", sf::Style::Close);
while (true)
{
std::getchar();
std::thread([&wnd]()
{
wnd.setActive(true);
wnd.clear(sf::Color::Blue);
wnd.display();
wnd.setActive(false);
// THIS ///////////////////////////////////////////////////////////////
sf::GlResource::releaseThreadResource();
// //////// ///////////////////////////////////////////////////////////////
}).join();
std::getchar();
wnd.setActive(true);
wnd.clear(sf::Color::Green);
wnd.display();
wnd.setActive(false);
}
return 0;
}
SFML snapshot: LaurentGomila-SFML-86897a8, downloaded 22nd of April 2013.
OS: Linux Mint 14 (MATE desktop)
Compiler: GNU GCC
IDE: Code::Blocks
Compiler flags: -std=c++0x -Wextra -Wall -g
Linker: -lpthread /usr/lib/libsfml-audio-d.so /usr/lib/libsfml-graphics-d.so /usr/lib/libsfml-window-d.so /usr/lib/libsfml-system-d.so
Please correct me if/where I am wrong so I can try and fix it.