SFML community forums
Help => General => Topic started by: P@u1 on October 09, 2011, 12:36:38 pm
-
Hi,
I just started a new project and this time I used the singleton pattern for my game class.
I first implemented it so that in the static deinitializers the Game object is destroyed which invokes the RenderWindow destructor.
This yields a crash:
Unhandled exception at 0x77bb1c1d in LockstepMario.exe: 0xC0000005: Access violation reading location 0xfeeefef6.
ntdll.dll!77bb1c1d()
[Frames below may be incorrect and/or missing, no symbols loaded for ntdll.dll]
> LockstepMario.exe!sf::priv::MutexImpl::Lock() Line 52 + 0xc bytes C++
LockstepMario.exe!sf::Mutex::Lock() Line 62 C++
LockstepMario.exe!sf::Lock::Lock(sf::Mutex & mutex) Line 39 C++
LockstepMario.exe!sf::GlResource::~GlResource() Line 72 + 0xd bytes C++
LockstepMario.exe!sf::Renderer::~Renderer() + 0x16 bytes C++
LockstepMario.exe!sf::RenderTarget::~RenderTarget() Line 53 + 0xb bytes C++
LockstepMario.exe!sf::RenderWindow::~RenderWindow() Line 60 + 0xe bytes C++
LockstepMario.exe!Game::~Game() Line 14 + 0x8 bytes C++
LockstepMario.exe!Game::`scalar deleting destructor'() + 0x2b bytes C++
LockstepMario.exe!std::default_delete<Game>::operator()(Game * _Ptr) Line 2068 + 0x2b bytes C++
LockstepMario.exe!std::unique_ptr<Game,std::default_delete<Game> >::_Delete() Line 2345 C++
LockstepMario.exe!std::unique_ptr<Game,std::default_delete<Game> >::~unique_ptr<Game,std::default_delete<Game> >() Line 2302 C++
LockstepMario.exe!`dynamic atexit destructor for 'Game::instance_''() + 0x28 bytes C++
LockstepMario.exe!doexit(int code, int quick, int retcaller) Line 567 C
LockstepMario.exe!exit(int code) Line 393 + 0xd bytes C
LockstepMario.exe!__tmainCRTStartup() Line 284 C
LockstepMario.exe!mainCRTStartup() Line 189 C
kernel32.dll!760d3677()
ntdll.dll!77b89f02()
ntdll.dll!77b89ed5()
(I'm using visual studio 2010).
I did some investigation and figured out that MutexImpl::myMutex has the address 0xfeefee which means that it is uninitialized.
But when I put a call to release the singleton in atexit, it works.
Here is some code:
int main(int argc, char** argv)
{
Game::Instance().Run();
return 0;
}
//new file here
class Game
{
public:
Game();
~Game();
void Run();
static Game& Instance()
{
if(!instance_)
{
instance_.reset(new Game());
//atexit(Destroy); when I add this lines, everything works!
}
return *instance_;
}
private:
void Init();
void MainLoop();
void HandleEvents();
void Update();
void Draw();
sf::RenderWindow window_;
static void Destroy(){instance_.release();}
static std::unique_ptr<Game> instance_;
};
The other methods don't do anything interesting, but I can post them if you want.
Note: I'm using SFML 2.0, but not the latest build atm.
-
You should not destroy SFML resources at global exit. Behaviour is undefined in this case (SFML has globals too, and you can't enforce them to be destroyed after yours).
-
ok, good to know.
Didn't know that sfml uses globals.
-
[...] (SFML has globals too, and you can't enforce them to be destroyed after yours).
That is most unfortunate. Why exactly does SFML need static (or do you _really_ mean global) objects?
-
[...] (SFML has globals too, and you can't enforce them to be destroyed after yours).
That is most unfortunate. Why exactly does SFML need static (or do you _really_ mean global) objects?
Let's just say that ATi sucks at making drivers.
-
Can we say something a little more descriptive? I'd like to make sure there really is no way around this.
-
Can we say something a little more descriptive? I'd like to make sure there really is no way around this.
Due to some idiotic design choices in ATi's drivers, SFML must use a global/static (not sure which) OpenGL state or people using ATi Graphics Cards will suffer freezing, aka the ATi Bug.
-
So the real issue is global access to some shared state? I'm fairly certain that can be achieved without running into the static initialization fiasco (modulo the fact that I don't know all the details here).
-
This is not exactly because of the ATI bug.
It's complicated to explain but trust me, there's nothing I can do to avoid using global variables. Even adding explicit init/cleanup functions in SFML wouldn't solve the problem -- it wouldn't work better with globals in user code.
But these globals are causing a bug in SFML (the default font is a global resource, so it has the same problem), so one day I'll have to find a solution somehow...
-
This is not exactly because of the ATI bug.
It's complicated to explain but trust me, there's nothing I can do to avoid using global variables. Even adding explicit init/cleanup functions in SFML wouldn't solve the problem -- it wouldn't work better with globals in user code.
But these globals are causing a bug in SFML (the default font is a global resource, so it has the same problem), so one day I'll have to find a solution somehow...
Remove the default font and force the user to load one (if necessary), is surely the best option?
-
Remove the default font and force the user to load one (if necessary), is surely the best option?
It would only solve this particular issue, I prefer to solve the root of the problem (the global OpenGL context).
And I think that the default font is a very useful feature, it won't be easy to convince me to remove it ;)
-
I must admit I'm pretty curious as to why globals are impossible to remove. Not that I dont' believe you... it's just I find it intriguing.
-
There's a special OpenGL context which is shared with all others. I must use a special one because only an inactive context can be shared.
There's a reference counter, managed by the GlResource base class, which controls the lifetime of this context.
So if you have a window, texture or font which is destroyed at global exit, this shared context will be destroyed at global exit as well. But this variable is managed in a thread-safe way, so it requires at least a mutex, global as well. I could force the mutex to be destroyed after the context, but that would require some ugly code and it's probably not the only problem (the context management code is rather complex).
-
After having a glance through source, I can't immediately see the need for having the global mutex you mentioned. It seems the following construct would be sufficient (note that I've stuffed all this inside GlResource simply because that is where the current code handles the initialization):
In GlResource.hpp:
namespace sf
{
class GlResource
{
public:
class EnsureSharedContext
{
public:
EnsureSharedContext();
~EnsureSharedContext();
private:
static unsigned myInclusionCount;
};
// ...
};
} // namespace sf
namespace
{
::sf::GlResource::EnsureSharedContext sfGlResourceEnsureSharedContextObject;
} // unnamed namespace
In GlResource.cpp:
#include <SFML/Window/GlResource.hpp>
#include <SFML/Window/GlContext.hpp>
// ...
namespace sf
{
unsigned GlResource::EnsureSharedContext::myInclusionCount = 0;
GlResource::EnsureSharedContext::EnsureSharedContext()
{
if (myInclusionCount++ == 0)
{
::sf::priv::GlContext::GlobalInit();
}
}
GlResource::EnsureSharedContext::~EnsureSharedContext()
{
if (--myInclusionCount == 0)
{
::sf::priv::GlContext::GlobalCleanup();
}
}
} // namespace sf
What do you think?
As an aside: I noticed that you create a function local static 'sf::Mutex' object in 'sf::priv::WglContext::CreateContext'. This isn't thread safe without some external synchronization (and I assume there isn't any, or the mutex would be redundant).
-
What I forgot to say is that I can't trigger the global init at global startup -- that would make the ATI bug come back.
So I really have to call this when the first GlResource is created.
And I'm not sure that your solution would solve the "global destruction fiasco" problem: what happens, for example, when a GlResource is allocated as a function-local static variable (such as the default font)? Is there a C++ rule that ensures that it is destroyed before globals that are instanciated in the same code unit?
Same for GlResource::EnsureSharedContext::myInclusionCount: it may be initialized after the sfGlResourceEnsureSharedContextObject globals are constructed. And it will, at least in GlResource.cpp, because the global instance is declared first in this code unit.
-
As an aside: I noticed that you create a function local static 'sf::Mutex' object in 'sf::priv::WglContext::CreateContext'. This isn't thread safe without some external synchronization (and I assume there isn't any, or the mutex would be redundant).
It's true. But I can't create it in the global scope -- I would have the exact same problem.
Let's wait until some unlucky user actually gets a problem with that :D
-
What I forgot to say is that I can't trigger the global init at global startup -- that would make the ATI bug come back.
So I really have to call this when the first GlResource is created.
Meaning, you cannot make driver calls before 'main()' has been entered?
And I'm not sure that your solution would solve the "global destruction fiasco" problem: what happens, for example, when a GlResource is allocated as a function-local static variable (such as the default font)? Is there a C++ rule that ensures that it is destroyed before globals that are instanciated in the same code unit?
The constructor for a function local static object is called at the first time execution passes through the block in which the object was defined. Destruction happens in the reverse order of construction (yes, even for function local static objects). So if an object was constructed before the first call to the function in which the font object is defined, it will outlive the font object, and vice-versa if the function was called first.
Same for GlResource::EnsureSharedContext::myInclusionCount: it may be initialized after the sfGlResourceEnsureSharedContextObject globals are constructed. And it will, at least in GlResource.cpp, because the global instance is declared first in this code unit.
No it wont. The initialization of objects with static storage duration is a two phase process. 'myInclusionCount' doesn't require any dynamic initialization (which is the second phase), while the 'sfGlResourceEnsureSharedContextObject' object does (i.e. its constructor must be run), so we're safe.
-
Meaning, you cannot make driver calls before 'main()' has been entered?
With Catalyst > 9.8 on Windows from a DLL, yes.
You cannot make driver calls after main as well, in all versions of the driver.
(well, it's not "any driver call", it's WGL calls related to construction/destruction of OpenGL contexts)
The constructor for a function local static object is called at the first time execution passes through the block in which the object was defined. Destruction happens in the reverse order of construction (yes, even for function local static objects). So if an object was constructed before the first call to the function in which the font object is defined, it will outlive the font object, and vice-versa if the function was called first.
Are you sure? I thought that this rule applied only to globals, and that the mix between function static and globals were undefined.
'myInclusionCount' doesn't require any dynamic initialization
Isn't "= 0" the dynamic initialization? Until that moment, the variable may contain random bits, doesn't it?
-
Are you sure? I thought that this rule applied only to globals, and that the mix between function static and globals were undefined.
Yup, I'm sure. I can even quote the standard if you like.
Isn't "= 0" the dynamic initialization? Until that moment, the variable may contain random bits, doesn't it?
Nope. It is static initialization because the initializer is a constant expression.
EDIT: And in fact, the variable would have been guaranteed to be initialized with "0" even without the explicit initializer.
-
Thanks for the clarification. I don't have a copy of the standard, so this kind of details are sometimes unclear to me :)
But we still have the problem of WGL calls before/after main() with ATI drivers.
-
But we still have the problem of WGL calls before/after main() with ATI drivers.
Noted.
It seems the only solution other than banning global SFML objects (which doesn't sound totally unreasonable), would be to decouple context management from the lifetime of individual resource objects. I'm not sure if I have the patience fully grok the SFML code base, so I shall leave that in your capable hands. ;)
-
It seems the only solution other than banning global SFML objects (which doesn't sound totally unreasonable)
It is the current solution, but it's definitely not convenient. SFML has globals (the default font), users too (image manager singleton, ...), it would be cool if it could work just fine.
would be to decouple context management from the lifetime of individual resource objects
It's complicated.
Resources require a context in order to be destroyed, so the only solution would be to keep a list of all the living resources, and deallocate their internal OpenGL resources when the last context is destroyed.
But this leads to a question: if resources no longer control the lifetime of the shared context, when should this one be destroyed? Destroying it when no other context exists is not an option, we would lose all the loaded OpenGL resources (textures, shaders, fonts).
sf::Texture texture;
texture.LoadFromFile("toto.png");
sf::Texture screen;
{
sf::RenderTexture surface;
surface.Create(256, 256);
surface.Clear();
surface.Draw(sf::Sprite(texture));
surface.Display();
screen = surface.GetTexture();
}
// here, both "texture" and "screen" would be lost...
sf::RenderWindow window;
...
That's why I made it this way: I'm sure that at least one OpenGL context is existing as long as there are OpenGL resources alive.
I'm aware that my requirements for SFML are pretty high, probably higher than other OpenGL-based libraries, but this is what makes the S of SFML.
-
Okay, I'll bite ;)
Again, this is all based on my very limited knowledge of SFML's internals:
1) Create a sf::GraphicsInitialize object which must be constructed as an automatic object (disable operator new/delete to enforce this) some time after main() has been entered. sf::GraphicsInitialize would be responsible for the creation and destruction of both the shared context and the default font (and whatever else is needed).
2) Remove any context creation/destruction code from GlResource.
3) Allow EnsureContext() to fail if called outside the lifetime of the sf::GraphicsInitialize object.
4) Resources treat 'EnsureContext()' like any other possibly failing function (e.g. assert, throw, return false, whatever), except from destructors, where failure to EnsureContext() implies that nothing need be done (i.e. no context implies no resource to destroy).
What do you think?
-
I think that would work, but it still doesn't allow anyone to have globals; which is almost equivalent to the current implementation (it would just provide a way for SFML to control the lifetime of its own globals).
Or do you meant that this must be combined with the "list of resources" solution that was mentioned before? And that destroying the GraphicsInitialize object releases all the active GlResource instances?
-
Resources require a context in order to be destroyed, so the only solution would be to keep a list of all the living resources, and deallocate their internal OpenGL resources when the last context is destroyed.
How does this solve the problem that the OpenGL context must be destroyed after main(), if you have global resource objects?
Or would it be enough if the context-related functionality were only used during the execution of main(), but the C++ object's lifetime were longer? I.e. the resource is invalid outside main()?
-
Or would it be enough if the context-related functionality were only used during the execution of main(), but the C++ object's lifetime were longer? I.e. the resource is invalid outside main()?
That was the idea, yes.
-
Okay. The only way to ensure that the OpenGL context is destroyed at the end of main() seems to be a RAII object as proposed by Lee R.
To destroy the graphics context slightly after main(), std::atexit() is a possibility. The C++ standard says, the registered functions are called at normal program termination. But I understand it correctly, the order of destroying global/static objects and invoking the atexit() callbacks is not defined, so this is of limited use here... Andrei Alexandrescu also wrote a function SetLongevity(T* dynamicObject, unsigned int longevity) which would destroy objects allocated with new in a specified and reliable order. With it, you could at least determine the destruction order of internally-used SFML objects, but not of user-defined global/static objects.
-
The only way to ensure that the OpenGL context is destroyed at the end of main() seems to be a RAII object as proposed by Lee R.
A non-RAII version would also be required, for bindings, plugins, etc.
-
I think that would work, but it still doesn't allow anyone to have globals; which is almost equivalent to the current implementation (it would just provide a way for SFML to control the lifetime of its own globals).
What it does is turn undefined behaviour into diagnosable errors: resource objects can be default constructed before main(), but operations requiring a valid context will return an error value, rather than cause the application to crash/hang/who-knows-what.
The rules cannot be more relaxed than that, since it is fundamentally impossible to create a resource either before or after the lifetime of a valid context. It's simple and it makes sense: you cannot use library functions before the library has been initialized or after it has been shut down; and in the Windows-ATI-DLL case, that initialization must be done some time after main() has entered.
Or do you meant that this must be combined with the "list of resources" solution that was mentioned before? And that destroying the GraphicsInitialize object releases all the active GlResource instances?
If that really is necessary, then yes. I was had got impression that destroying a context implicitly freed its resources, though?
-
I was had got impression that destroying a context implicitly freed its resources, though?
I'm not sure. Resources are lost for sure, but I don't know if they are properly released.