Window::waitEvent does not return when window is closed

[norgor]

Description

Window::waitEvent does not return after Window::close is called, which causes the eventual dedicated event thread to wait for an event that will never arrive.

Solution

bool WindowImpl::popEvent(Event& event, bool block)
{
...
        // In blocking mode, we must process events until one is triggered
        if (block)
        {
            // Here we use a manual wait loop instead of the optimized
            // wait-event provided by the OS, so that we don't skip joystick
            // events (which require polling)
            while (m_events.empty())
            {
                sleep(milliseconds(10));
		
		//Check if window is open

                processJoystickEvents();
                processSensorEvents();
                processEvents();
            }
        }
    }
...
}

[eXpl0it3r]

We like to discuss unconfirmed issues and design questions (like this one here) on the forum as stated in the contribution guidelines.

While I at first thought that this might need adjustments to the documented or adding your suggested // Check if window is open, I wonder on second thought, whether this is intentional.

Sure, if you close the window, ignore the close event and then call waitEvent again, your thread will get stuck in an infinite loop, but this is essentially a programming error. And as long as the function returns again after you re-create a new window (this needs to be tested!) it's not even an unrecoverable infinite loop.

A similar situation can be constructed for pollEvent: If you use your own game-loop condition (e.g. while(gameIsRunning)) and don't set the flag to false when the close event gets triggered, then you'll end up with an infinite loop as well. The only difference is, that you can control this loop more directly (user code), while for waitEvent the loop occurs within SFML (library code) itself.

My conclusion

It's working as intended (test for the window re-creation situation is needed) and if you get stuck in an infinite loop, it's a programming error that can easily be prevented.

[norgor]

AFAIK the problem is that the queue gets freed upon closing the window, therefore the time window for Window::waitEvent to catch the event successfully is very small (if an event does get pushed). When I am talking about closing the window I mean using an in-application button to call Window::close therefore there would be no event beforehand.

When the window has been closed and the thread is inside Window::waitEvent, it continues to loop indefinetly.

[LaurentGomila]

SFML classes are not thread-safe (and I hope it is documented properly). So you have to take care of concurrent access, such as closing a window from one thread while the other thread is processing events.

Maybe sf::Window could be made thread-safe, since if you use threads, it will most likely involve this class. But to be 100% safe, it may require a lot of work and we have to evaluate the performance cost.

[eXpl0it3r]

Okay with that view, things seem to look different. (Maybe next time provide the real problem first 😉).

SFML classes are not thread-safe. So you have to take care of concurrent access, such as closing a window from one thread while the other thread is processing events.

I so far never questioned it, but does SFML's implementation guarantee that the close event will always be triggered before the impl get fully destroyed up on calling close()?

If that guarantee is not given, then there's nothing a user can do, to make the use of close() in combination with waitEvent "thread-safe".

close() will destroy the impl object, which in turn should trigger a close event. Question is, whether the specific OS implementations guarantee that the event is processed before the impl object is destroyed. But since we do a 10ms sleep in the waitEvent loop, I feel like this is a race condition that can easily lead to a locked thread.

The next question would then be, if close() can not be used in a thread-safe environment with waitEvent, how do you programmatically close your window/application then? Or how would one go about making close() thread-safe again?

[LaurentGomila]

Since waitEvent() is blocking, there's nothing a user can do. The typical solution would be to do like SFML does internally: call pollEvent in a loop with a small sleep between each itreation. From there you can use synchronization primitives (like mutexes) to protect things.

But I agree that something should be done internally to avoid this kind of issue.

[eXpl0it3r]

What's the use case then for waitEvent() if it effectively can't be used in a multi-threaded environment?`

The only time you can use waitEvent() "safely" in multi-threaded environment is, when you can ensure that the OS triggers a close event (by either the user clicking the X or Alt+F4). This is not really practical and if we do want to keep that constraint, then we should either document this and/or remove the hint in the tutorial for waitEvent()'s usage.

The only other use case I can kind of think of, is an application that is purely based on waiting for events. So it only does something if there are events. Which seems like a really odd use case.

With that in mind, I feel we should make it possible for waitEvent() to break free, or to add a guarantee that the close event will be triggered before the window gets "closed".

[binary1248]

What's the use case then for waitEvent() if it effectively can't be used in a multi-threaded environment?`

The only reason I can can see right now why it can't be safely used in a multi-threaded environment is because we manage events using our own additional event queue. If we didn't have to periodically take care of sensors and joysticks we could block on the OS and not have to care. Mixing joystick/sensor handling with OS window event handling is something that could be separated in the future. It is an unnecessary burden that we put on users who don't intend to make use of those features.

The only time you can use waitEvent() "safely" in multi-threaded environment is, when you can ensure that the OS triggers a close event (by either the user clicking the X or Alt+F4). This is not really practical and if we do want to keep that constraint, then we should either document this and/or remove the hint in the tutorial for waitEvent()'s usage.

Or... if we sent out a "loopback event" on Window::close() that triggers the OS to send some kind of event. Other libraries have support for sending custom OS events, among other reasons for cases like this.

The only other use case I can kind of think of, is an application that is purely based on waiting for events. So it only does something if there are events. Which seems like a really odd use case.

In fact... at work I had to write among other things a GUI frontend for industrial machine control that would run with minimal resources on an embedded platform (RPi CM). It would draw the user interface and wait for user input or some other GUI-changing event to occur before proceeding with the next iteration through the main loop. I used GLFW (needed robust OpenGL ES 2.0 support and I used raw OpenGL all over the place anyway) and ended up just using glfwWaitEvents() and glfwPostEmptyEvent() for this. It ended up in a clean, very light-weight (0% CPU/GPU in over 99% of the time) and satisfactory solution.

With that in mind, I feel we should make it possible for waitEvent() to break free, or to add a guarantee that the close event will be triggered before the window gets "closed".

I think the main problem right now is firstly that waitEvent() busy waits without any internal synchronization and secondly that there is no way to get it out of the loop even if we wanted to from another thread. Like I mentioned above, I don't think adding support for some sort of user-defined or "empty" event would be a bad idea. It would give waitEvent() more reason to exist on its own, considering that ironically, just as if the user would periodically poll themselves, it internally polls anyway.

[eXpl0it3r]

I added some labels but I'm not really sure where we ended up with the discussion.

• SFML isn't thread-safe
• waitEvent() and close() in its current state can't be used in a multi-threaded environment
• SFML uses a "custom" waitEvent loop

Do we do something about this or do we just mark it as "won't fix"?