Possible Bug in RenderWindow Destructor (SFML 2.0)

[Laurent]

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

[Lee R]

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.

[Laurent]

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?

[Lee R]

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.

[Laurent]

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.

[Lee R]

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.

[Laurent]

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

Code: [Select]

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.

[Lee R]

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?

[Laurent]

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?

[Nexus]

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()?

[Laurent]

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.

[Nexus]

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.

[Laurent]

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.

[Lee R]

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?

[Laurent]

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.