Fix crash in glfw backend when backend data is null

[cpichard]

Hi,
This crash happens when my application loads a proprietary plugin, which hijacks the window context to display it's own UI.
When the plugin is running, the imgui callbacks are still called with a null backend data which makes the code using it crash. When the proprietary plugin closes its UI, the backend data is available again.
Checking if the backend data pointer is null and returning early fixes this issue.
Let me know if that sounds like a reasonable fix.
Cheers,
Cyril

[ocornut]

I don’t understand your explanation. Why would the backend data suddenly become null with callbacks installed ? Why not removing callbacks if you deinit the backend ?

[cpichard]

I have spent more time debugging this issue and hopefully it will make more sense:
There are 2 imgui contexts in the application, one for the UI, correctly initialized with glfw backend data, and another one to draw gizmos in a frame buffer using the imgui api, the later one doesn't have io.BackendPlatformUserData initialized.

The render loop code goes like this :

// Render image in frame buffer
ImGui::SetCurrentContext(gizmoContext);
render3dFrameBuffer();

// Render GUI next
ImGui::SetCurrentContext(mainUIContext);
glfwGetFramebufferSize(window, &width, &height);
glViewport(0, 0, width, height);
glClear(GL_COLOR_BUFFER_BIT | GL_DEPTH_BUFFER_BIT);
ImGui_ImplOpenGL3_NewFrame();
ImGui_ImplGlfw_NewFrame();
ImGui::NewFrame();
DrawUI();
ImGui::Render();
ImGui_ImplOpenGL3_RenderDrawData(ImGui::GetDrawData());

It worked well until a proprietary plugin created a window in the middle of render3dFrameBuffer.
It called glfw callbacks with ImGui::GetIO().BackendPlatformUserData == nullptr and crashed the application.

My first reflex was to fix this issue by testing the pointer for being null, specially because ImGui_ImplGlfw_GetBackendData looks like it can return nullptr, hence it doesn't assume the pointer is always valid, and this can potentially crash later on.

static ImGui_ImplGlfw_Data* ImGui_ImplGlfw_GetBackendData()
{
    return ImGui::GetCurrentContext() ? (ImGui_ImplGlfw_Data*)ImGui::GetIO().BackendPlatformUserData : nullptr;
}

I think it wouldn't be crazy to assume that BackendPlatformUserData can be null, specially when there are multiple contexts. This PR add tests for BackendPlatformUserData , returning early if it is null.

I am not 100% sure this is the best solution, so as an alternative, I have a fix working installing and restoring glfw callbacks at every frame as you suggested.

Best,

C

[ocornut]

the later one doesn't have io.BackendPlatformUserData initialized.

But why ?

[cpichard]

The second context is used to draw gizmos in an opengl offscreen render buffer which is then added as an image in the main ui.
The second context is only used for the drawing functions, to draw curves, lines, text in a viewport at a different time than the main UI. It doesn't make sense to have interaction here, the interaction is managed by the first context in the main UI.

So io.BackendPlatformUserData is null because there is no need of backend in that particular case.

Best,

C

[ocornut]

You said:

It worked well until a proprietary plugin created a window in the middle of render3dFrameBuffer.

It called glfw callbacks with ImGui::GetIO().BackendPlatformUserData == nullptr If the context is not meant to be used with a backend it should be bounds when backend callbacks are setup.

[cpichard]

Thanks,
like I said, I also have a solution installing and removing the callbacks at every frame, thanks to your first suggestion, so all good.
If you feel it is not worth testing the pointer returned by ImGui_ImplGlfw_GetBackendData() before using it, feel free to close this PR.
Thanks again for your quick replies and the help,
Best,
C

[ocornut]

If you feel it is not worth testing the pointer returned by ImGui_ImplGlfw_GetBackendData() before using it, feel free to close this PR.

My stance is that those those asserts/crashes are valuable and desirable ways to spot misusage, and that switching to "silent erroring" patterns is rarely desirable. While adding a NULL check would be convenient for our use case, it may make it more difficult for other users to spot other genuine mistakes.

I would generally accept the null checks if it was proved it was the only situation to a tricky problem.
Right now, even if more convoluted, given your use case it seems more adequate to not trigger the callback.

I'll close this for now. Thanks for the details.

[cpichard]

That sounds good, thanks again for the quick help.
Best,
Cyril