SDL_WaitEventTimeout can block indefinitely on Wayland

[fstanis]

I was debugging some code in FreeRDP that seems to sometimes have huge input delays and found out that SDL_WaitEventTimeout seems to be the culprit. I have something that's similar to this loop (from FreeRDP):

while (!sdl->shallAbort())
    {
    SDL_Event windowEvent = {};
    while (!sdl->shallAbort() && SDL_WaitEventTimeout(nullptr, 1000))
    {
            ...
    }
}

It seems to happen when the cursor changes, specifically there seems to be a race condition in Wayland_CursorStateSetCursor.

1. The main thread enters Wayland_WaitEventTimeout, flushes, and blocks in poll on the display fd.
2. A cursor change (e.g., from SDL_SetCursor called from another thread, or from an SDL main-thread callback) queues Wayland requests in the write buffer.
3. Neither Wayland_ShowCursor() nor Wayland_CursorStateSetCursor calls wl_display_flush or Wayland_SendWakeupEvent.
4. The cursor thread is also blocked with infinite timeout (timeoutNS is -1 initially) on the same fd.
5. The compositor never receives the cursor requests, so it sends no events and poll never returns.

Probably the simplest thing to do would be to flush cursor events in Wayland_CursorStateSetCursor or alternatively to call Wayland_SendWakeupEvent to avoid a deadlock.

[Kontrabant]

So I understand this correctly, you expect SDL_WaitEventTimeout() to break out of the wait when calling SDL_SetCursor() from another thread? SDL_WaitEventTimeout() only breaks out of the wait if an SDL event becomes available in the queue, and setting the cursor doesn't generate any events, so it won't return, even if a flush is added in the cursor update code.

SDL_SetCursor() should also only be called from the main thread, as is noted in the docs.

[Kontrabant]

Take this example: upon releasing the 'C' key, SDL_SetCursor() will be called from a separate thread after 2 seconds, but, even when adding a flush to the internal cursor setting code, SDL_WaitEvent() won't break out of the wait, since no SDL events were generated, but still returns every 10 seconds, when the timeout expires. This is the correct behavior.

#include <SDL3/SDL.h>

static SDL_Cursor *arrow;
static int SDLCALL SetCursorThread(void *data)
{
    SDL_Delay(2000);
    SDL_SetCursor(arrow);
    SDL_Log("Cursor changed");

    return 0;
}

int main(int argc, char **argv)
{
    SDL_InitSubSystem(SDL_INIT_VIDEO);

    SDL_Renderer *renderer;
    SDL_Window *window;

    arrow = SDL_CreateSystemCursor(SDL_SYSTEM_CURSOR_CROSSHAIR);

    SDL_CreateWindowAndRenderer("Test", 1280, 720, 0, &window, &renderer);

    int i = 0;
    for (;;) {
        SDL_Event e;
        const bool ret = SDL_WaitEventTimeout(&e, 10000);
        if (ret) {
            if (e.type == SDL_EVENT_WINDOW_CLOSE_REQUESTED || e.type == SDL_EVENT_QUIT) {
                break;
            } else if (e.type == SDL_EVENT_WINDOW_EXPOSED) {
                SDL_SetRenderDrawColor(renderer, 0, 0, 0, 255);
                SDL_RenderClear(renderer);
                SDL_RenderPresent(renderer);
            } else if (e.type == SDL_EVENT_KEY_UP && e.key.key == SDLK_C) {
                SDL_CreateThread(SetCursorThread, "testthread", NULL);
            }
            SDL_Log("Got event %i", i++);
        } else {
            SDL_Log("Wait timed out %i", i++);
        }
    }

    SDL_DestroyWindow(window);
    SDL_DestroyRenderer(renderer);

    SDL_Quit();
    return 0;
}

[fstanis]

Thanks for the response and apologies I didn't clarify - FreeRDP does call SDL_SetCursor from the main thread, specifically a network event (cursor change) calls SDL_PushEvent, so the cursor is changed from the event handler.

I tried to create a minimal repro outside FreeRDP, but failed, so I'm no longer so sure my analysis was correct. What I do know for sure is that a call to SDL_WaitEventTimeout blocked for more than 30 seconds despite the timeout being set to 1 second, specifically it was this call here:

https://github.com/FreeRDP/FreeRDP/blob/1d3c2d0b69ea51717cfa2a176ef9d45cc8e07301/client/SDL/SDL3/sdl_freerdp.cpp#L171

Based on traces, the last successful message that the event handler processed before this was one that sets the cursor, specifically sdl_Pointer_Set_Process here:

https://github.com/FreeRDP/FreeRDP/blob/1d3c2d0b69ea51717cfa2a176ef9d45cc8e07301/client/SDL/SDL3/sdl_pointer.cpp#L114

I know linking to a different project's code (that I'm not a maintainer of) is not super helpful and probably we won't make much progress without a consistent repro, but I wanted to at least leave this information here, in case someone else comes across a similar issue.

[Kontrabant]

Thanks for clarifying.

Yes, figuring out exactly what is happening here without a detailed trace or reproducer is going to be very difficult. At least on Wayland, the timeout value only controls how long the poll call waits for system events to be available on the socket. Otherwise, the event pump is basically the same as the reference implementations. If FreeRDP is enqueuing user events from another thread, the event queue locks are involved as well. Any number of things could be happening here.