[2.6.1] segfault in waitEvent on close (Ubuntu/X11/Multithreading)

Question

[2.6.1] segfault in waitEvent on close (Ubuntu/X11/Multithreading)

frayien opened this issue 4 months ago · 7 comments

Prerequisite Checklist

I searched for existing issues to prevent duplicates
I searched for existing discussions on the forum to prevent duplicates
I am here to report an issue and not to just ask a question or look for help (use the forum or Discord instead)

Describe your issue here

On application close (on call to sf::RenderWindow#close in render thread), SFML segfault on sf::RenderWindow#waitEvent (or pollEvent) in main thread.

Said otherwise, while waitEvent waits in thread A, if a RenderWindow#stop is called in a thread B, waitEvent causes a segfault.

NOTED :
In this specific case breaking the event loop after sf::Event::Close is enough, but it is not applicable in every situation because the event loop may not control when a RenderWindow#close is called.

Also working around it (by manually stopping the event loop before calling RenderWindow#stop) seems not possible reliably because by nature the thread is waiting in waitEvent.

Your Environment

OS / distro / window manager: Ubuntu 22.04.3 LTS
SFML version: 2.6.1
Compiler / toolchain: gcc 11.4.0 (Ubuntu 11.4.0-1ubuntu1~22.04) cmake 3.22.1 ninja 1.10.1
Special compiler / CMake flags: (SFML built with FetchContent)
- set(CMAKE_CXX_STANDARD 23)
- set(CMAKE_CXX_EXTENSIONS OFF)
- set(CMAKE_CXX_STANDARD_REQUIRED TRUE)
- set(CMAKE_CXX_FLAGS "-Wall -Wextra")
- set(CMAKE_CXX_FLAGS_DEBUG "-Og -g")
- set(CMAKE_CXX_FLAGS_RELEASE "-O3 -g1")
- set(BUILD_SHARED_LIBS OFF)
- set(SFML_BUILD_EXAMPLES OFF)
- set(SFML_BUILD_DOC OFF)
- set(SFML_BUILD_NETWORK OFF)
- set(SFML_BUILD_AUDIO ON)
- set(SFML_BUILD_GRAPHICS ON)
- set(SFML_BUILD_WINDOW ON)
- target_link_libraries(${PROJECT_NAME} sfml-graphics sfml-window sfml-system sfml-audio)

Steps to reproduce

Compile code, run it and close the window

CMake :

cmake_minimum_required(VERSION 3.21.0)
include(FetchContent)

project(Demo)

### CMake config ###
set(CMAKE_EXPORT_COMPILE_COMMANDS ON)

if(NOT CMAKE_BUILD_TYPE)
  set(CMAKE_BUILD_TYPE Release)
endif()

message(STATUS "GCC detected, adding compile flags")
set(CMAKE_CXX_FLAGS "-Wall -Wextra")
set(CMAKE_CXX_FLAGS_DEBUG "-Og -g")
set(CMAKE_CXX_FLAGS_RELEASE "-O3 -g1")

set(CMAKE_CXX_STANDARD 23)
set(CMAKE_CXX_EXTENSIONS OFF)
set(CMAKE_CXX_STANDARD_REQUIRED TRUE)

### Dependency declaration ###

FetchContent_Declare(
  sfml
  GIT_REPOSITORY "https://github.com/SFML/SFML.git"
  GIT_TAG        2.6.1
)

### Dependency population ###

set(BUILD_SHARED_LIBS OFF)

# sfml
set(SFML_BUILD_EXAMPLES OFF)
set(SFML_BUILD_DOC OFF)

set(SFML_BUILD_NETWORK OFF)

set(SFML_BUILD_AUDIO ON)
set(SFML_BUILD_GRAPHICS ON)
set(SFML_BUILD_WINDOW ON)
# !sfml

FetchContent_MakeAvailable(sfml)

set(SRCS ${CMAKE_CURRENT_LIST_DIR}/test.cpp)

add_executable(${PROJECT_NAME} ${SRCS})

target_link_libraries(
  ${PROJECT_NAME}
  sfml-graphics
  sfml-window
  sfml-system
  sfml-audio
)

Source :

#include <SFML/Graphics.hpp>

#include <thread>
#include <atomic>
#include <X11/Xlib.h>

int main()
{
    XInitThreads();
    sf::RenderWindow win(sf::VideoMode(640, 480), "Title");

    std::atomic_bool fence(false);
    win.setActive(false);

    std::thread th([&](){
        win.setActive(true);

        while(win.isOpen())
        {
            win.clear(sf::Color(0x1a5e21FF));
            win.display();

            if(fence.exchange(false))
            {
                win.close();
            }
        }
    });

    while(win.isOpen())
    {
        sf::Event event;
        while(win.waitEvent(event))
        {
            if(event.type == sf::Event::Closed)
                fence.store(true); 
        }
    }

   th.join();
}

Expected behavior

Do not segfault

Actual behavior

Console output :

Setting vertical sync not supported
Erreur de segmentation (core dumped)

(Not sure what causes "Setting vertical sync not supported" but it is not the subject here)

Core dump (waitEvent) :

(gdb) bt
#0  0x0000000000000000 in ?? ()
#1  0x0000564aa98b678d in sf::priv::WindowImpl::popEvent (this=0x564aaa2739d0, 
    event=..., block=block@entry=true)
    at /home/frayien/Documents/build/_deps/sfml-src/src/SFML/Window/WindowImpl.cpp:160
#2  0x0000564aa98b5ba1 in sf::WindowBase::waitEvent (
    this=this@entry=0x7ffc2ea180f0, event=...)
    at /home/frayien/Documents/build/_deps/sfml-src/src/SFML/Window/WindowBase.cpp:179
#3  0x0000564aa987b1b9 in main () at /home/frayien/Documents/FNES/test.cpp:33

Core dump (pollEvent) :

(gdb) bt
#0  0x00007f0be88228be in XUnsetICFocus ()
   from /lib/x86_64-linux-gnu/libX11.so.6
#1  0x0000563ddd9ab0f9 in sf::priv::WindowImplX11::processEvent ()
    at /home/frayien/Documents/build/_deps/sfml-src/src/SFML/Window/Unix/WindowImplX11.cpp:1724
#2  0x0000563ddd9abb63 in sf::priv::WindowImplX11::processEvents ()
    at /home/frayien/Documents/build/_deps/sfml-src/src/SFML/Window/Unix/WindowImplX11.cpp:777
#3  0x0000563ddd9a5e3c in sf::priv::WindowImpl::popEvent ()
    at /home/frayien/Documents/build/_deps/sfml-src/src/SFML/Window/WindowImpl.cpp:147
#4  0x0000563ddd9a45d0 in sf::WindowBase::pollEvent ()
    at /home/frayien/Documents/build/_deps/sfml-src/src/SFML/Window/WindowBase.cpp:165
#5  0x0000563ddd9502db in main () at /home/frayien/Documents/FNES/test.cpp:33

Answer 1 · 2024-01-28T15:50:18.000Z

Neither sf::RenderWindow, nor sf::Window, nor sf::WindowBase are thread-safe. As such polling events in one and closing the window in another thread can/will lead to race conditions and potential crashes.

If you really want to have two different threads access the window, you need to properly synchronize the access to it between the two threads. Not that I'd recommend to use multiple threads to begin with.

Answer 2 · 2024-01-28T16:22:45.000Z

I see, noteworthy that I inherit this way of handling things from the section Drawing from threads of the tutorial ...

Tutorial does no expend on how to handle closing tho, which is the tricky part here.

So drawing in one thread an polling in another is supported, but closing I shall take special measures on my side to handle ?

Hmm, I guess to prevent any error the best approach would be to stop the render thread and then call stop in the event thread somehow.

I guess that's ok, but feels weird and counterintuitive. Maybe the tutorial should have a note in the section "Drawing from threads" to explain that it showing render and events in two different threads is an exception thread-safetywise and not the rule.

Answer 3 · 2024-01-28T16:31:15.000Z

Note that taking the code example from Drawing from threads and slapping the code from Bringing the window to life in it to handle events does not feel like a thing that's farfetched and wrong to a newcomers.

void renderingThread(sf::RenderWindow* window)
{
    // activate the window's context
    window->setActive(true);

    // the rendering loop
    while (window->isOpen())
    {
        // draw...

        // end the current frame
        window->display();
    }
}

int main()
{
    // create the window (remember: it's safer to create it in the main thread due to OS limitations)
    sf::RenderWindow window(sf::VideoMode(800, 600), "OpenGL");

    // deactivate its OpenGL context
    window.setActive(false);

    // launch the rendering thread
    sf::Thread thread(&renderingThread, &window);
    thread.launch();

    // the event/logic/whatever loop
    while (window.isOpen())
    {
        // check all the window's events that were triggered since the last iteration of the loop
        sf::Event event;
        while (window.pollEvent(event))
        {
            // "close requested" event: we close the window
            if (event.type == sf::Event::Closed)
                window.close();
        }
    }

    return 0;
}

PS : the error here is "Failed to activate OpenGL context: La ressource demande est en cours dutilisation." which is just the other side of the coin relative to the segfault in event.

Answer 4 · 2024-01-28T20:44:37.000Z

I see, noteworthy that I inherit this way of handling things from the section Drawing from threads of the tutorial ...

Tutorial does no expend on how to handle closing tho, which is the tricky part here.

So drawing in one thread an polling in another is supported, but closing I shall take special measures on my side to handle ?

Hmm, I guess to prevent any error the best approach would be to stop the render thread and then call stop in the event thread somehow.

I guess that's ok, but feels weird and counterintuitive. Maybe the tutorial should have a note in the section "Drawing from threads" to explain that it showing render and events in two different threads is an exception thread-safetywise and not the rule.

As you say the docs probably do leave a lot to be desired regarding working with mutliple threads & SFML. I myself learned a lot of lessons regarding this by just encountering problems. Maybe they could be expanded to better inform the user, but I think the essential core here is that you cannot presume any method or class SFML is exposing is thread-safe unless explicitly stated. If you do anything which involves reading state on one thread, whilst manipulating it on another then you're at risk because SFML has not given you any assurances.

Answer 5 · 2024-01-28T21:10:37.000Z

Yep ! Guess I'll just close this issue then. Should I open an issue on SFML-Website and submit some PR to https://github.com/SFML/SFML-Website/blob/master/tutorials/2.6/graphics-draw.php or is it out of scope as focus has shifted to SFML 3 ?

Answer 6 · 2024-01-29T05:54:21.000Z

Yeah, adding some hint regarding thread-safety on the website is a good idea 👍

Answer 7 · 2024-01-29T18:36:55.000Z

Found this issue on the website about thread-safety : SFML/SFML-Website#127
I'll take it as a basis to propose a PR, I'll probably come to the Discord if I have questions and/or for feedback.

Closing this as "Not a bug"