-
-
Notifications
You must be signed in to change notification settings - Fork 11.7k
Thread-safety of handling input events #6895
Description
Version/Branch of Dear ImGui:
Version: 28b237f (master at the time of writing)
Branch: master
Back-end/Renderer/Compiler/OS
Back-ends: SDL3+Vulkan
Compiler: GCC
Operating System: Linux
My Issue/Question:
I just implemented ImGui in one of my applications, which unfortunately handles input events (from SDL) in a separate thread from the one who is calling ImGui::NewFrame().
This is results in an issue since ImGui::NewFrame will call ImGui::UpdateInputEvents(...) which in turn interacts with InputEventsQueue
Lines 9032 to 9035 in 28b237f
| if (event_n == g.InputEventsQueue.Size) | |
| g.InputEventsQueue.resize(0); | |
| else | |
| g.InputEventsQueue.erase(g.InputEventsQueue.Data, g.InputEventsQueue.Data + event_n); |
This means that if NewFrame is invoked asynchronously that there is a possibility for a crash, since most the input event "receivers" will call FindLatestInputEvent at some point, which then queries the size of this event queue and then looks up all events from size to 0 (in vector index)
That means that if the following happens, there is a guaranteed crash:
static ImGuiInputEvent* FindLatestInputEvent(ImGuiContext* ctx, ImGuiInputEventType type, int arg = -1)
{
ImGuiContext& g = *ctx;
for (int n = g.InputEventsQueue.Size - 1; n >= 0; n--) // --- n is set to a random value that is likely to be >0
{ // --- UpdateInputEvents invokes resize or erase here
ImGuiInputEvent* e = &g.InputEventsQueue[n]; // --- Since InputEventsQueue has been cleared, we access out of bounds memory hereI solved this issue by simply putting an lock/mutex around NewFrame and the implementations process-event-function, however I think there should be I more appropriate solution to this. I think about something like a flag that when set will cause the next event receiver to do the said operations in their thread.
I am sorry if thread-safety is not considered ImGuis job, in that case simply close the issue.