Possible Bug in RenderWindow Destructor (SFML 2.0)

[P@u1]

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:

Code: [Select]

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.

[Laurent]

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).

[P@u1]

ok, good to know.
Didn't know that sfml uses globals.

[Lee R]

[...] (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?

[OniLinkPlus]

[...] (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.

[Lee R]

Can we say something a little more descriptive? I'd like to make sure there really is no way around this.

[OniLinkPlus]

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.

[Lee R]

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).

[Laurent]

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...

[slotdev]

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?

[Laurent]

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

[Disch]

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.

[Laurent]

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).

[Lee R]

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:

Code: [Select]


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:

Code: [Select]

#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).

[Laurent]

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.