Crash in _rmt_UnbindOpenGL

[marcakafoddex]

I am using:

• Windows 10
• Microsoft Visual Studio 2019 Preview 4.0
• 64-bits build
• Debug build

I build remotery (and the rest of my code) statically with the following defines:

• RMT_USE_OPENGL=1
• RMT_ENABLED=1

There are two scenarios.

SCENARIO 1:
This works fine.

1. Main thread initializes Remotery: rmt_CreateGlobalInstance(&rmt);
2. Various threads are started that do work using rmt_ScopedCPUSample. One of those threads is a renderer thread, which does rmt_BindOpenGL after activating the context, then uses rmt_ScopedOpenGLSample, and finally when shutting down calls rmt_UnbindOpenGL, and the deactivates the OpenGL context
3. When all threads are gone but the main thread, the main thread calls rmt_DestroyGlobalInstance(rmt);

SCENARIO 2:
In this scenario the crash happens.

1. Main thread initializes Remotery: rmt_CreateGlobalInstance(&rmt);
2. Various threads are started that do work using rmt_ScopedCPUSample. One of those threads is a renderer thread, which does rmt_BindOpenGL after activating the context, then uses rmt_ScopedOpenGLSample, and finally when shutting down calls rmt_UnbindOpenGL, and the deactivates the OpenGL context
3. A new set of threads is started, with a new renderer, but reusing the old OpenGL context
4. As before in step 2, one of the new threads is the renderer thread, so it makes the OpenGL context current, calls rmt_BindOpenGL, and starts rendering. When shutting down, as before in 2, it calls rmt_UnbindOpenGL and then deactivates the OpenGL context.
5. When all threads are gone but the main thread, the main thread calls rmt_DestroyGlobalInstance(rmt);

In this second scenario, the crash happens in the second rmt_UnbindOpenGL call in step 4, which is the second time I'm calling it. The call relevant callstack is short:

!FlattenSampleTree(Sample * sample, unsigned int * nb_samples) Line 4237 C
!FlattenSampleTree(Sample * sample, unsigned int * nb_samples) Line 4242 C
!FreeSampleTree(Sample * sample, ObjectAllocator * allocator) Line 4260 C
!FreePendingSampleTrees(Remotery * rmt, SampleType sample_type, Buffer * flush_samples) Line 5520 C
!_rmt_UnbindOpenGL() Line 6891 C

This is in the following code:

static ObjectLink* FlattenSampleTree(Sample* sample, rmtU32* nb_samples)
{
    Sample* child;
    ObjectLink* cur_link = &sample->Link;

    assert(sample != NULL);
    assert(nb_samples != NULL);

    *nb_samples += 1;
    sample->Link.next = (ObjectLink*)sample->first_child;  // CRASH HERE (!!!)

    // Link all children together
    for (child = sample->first_child; child != NULL; child = child->next_sibling)
    {
        ObjectLink* last_link = FlattenSampleTree(child, nb_samples);
        last_link->next = (ObjectLink*)child->next_sibling;
        cur_link = last_link;
    }

    // Clear child info
    sample->first_child = NULL;
    sample->last_child = NULL;
    sample->nb_children = 0;

    return cur_link;
}

The exception thrown is on the line marked "// CRASH HERE (!!!)"

Exception thrown: read access violation. sample was 0xFFFFFFFFFFFFFFD7.

According to the debugger, the Sample* sample variable going in is 0xdddddddddddddddd, indicating the memory was already freed.

It seems somehow rmt_UnbindOpenGL doesn't expect to be called more than once?

[dwilliamson]

Following the code that seems rather impossible as the OpenGL sample tree is destroyed during unbind. It's then recreated on the next sample push.

Do you have any outstanding sample begin/end calls that haven't closed before the unbind?

[marcakafoddex]

There is only one thread that does OpenGL related calls do Remotery, so when unbind is called there is definitely no OpenGL related sampling being done anymore.

It IS in theory possible that non-OpenGL related sampling is still being done when OpenGL unbind is called. Could that be a problem?

Also, do I HAVE to make the unbind/bind opengl remotery calls on the same thread that does the render sampling, or can I do it from any thread?

[dwilliamson]

Unbind, at least, has to be called from the same thread that issues the samples. This is because each sample allocates an OpenGL query. When you unbind those queries are released and calling into the OpenGL API from multiple threads on the same context is undefined behaviour.

When I mean outstanding samples, sometimes this can happens:

BeginSample
Unbind
EndSample

It's the only thing I can think of right now but I've only given the code a cursory glance. I'll to a deeper walk through it later and maybe setup a test case.

[marcakafoddex]

Nope absolutely no OpenGL sampling going on.

I'm debugging it a bit. When I step into FreePendingSampleTrees the second time round, it gets into the while (data < dat_end) loop. On line 5518 (my code) it gets the sample tree from the payload:

Msg_SampleTree* sample_tree = (Msg_SampleTree*)message->payload;

This sample_tree has 3 members, root_sample, allocator and thread_name. The second two look perfectly fine in the debugger, but the root_sample points to deleted memory. Literally every member in that root_sample has 0xdddddddddd as its value, MSVC's way of telling you the memory was freed. Hope this helps.

[marcakafoddex]

Did a little more research, the values in the sample_tree can become pretty much anything. I sprinkled the code with IsBadReadPtr checks on Windows, got random hits on that during the second UnbindOpenGL -> FreePendingSampleTrees call everywhere.

[dwilliamson]

OK, got a repro here:

#include <stdio.h>

#include <windows.h>
#include <gl/gl.h>
//#include <gl/glu.h>
#include "../lib/Remotery.h"


#pragma comment(lib, "user32.lib")
#pragma comment(lib, "gdi32.lib")
#pragma comment(lib, "opengl32.lib")
//#pragma comment(lib, "glu32.lib")


HINSTANCE hInstance;
HWND hWnd;
HDC hDC;
HGLRC hRC;


int InitOpenGL()
{
    PIXELFORMATDESCRIPTOR pfd;
	int pixel_format;

	// Set the default pixel format
	ZeroMemory(&pfd, sizeof(PIXELFORMATDESCRIPTOR));
	pfd.cColorBits = 32;
	pfd.cDepthBits = 32;
	pfd.dwFlags = PFD_SUPPORT_OPENGL | PFD_DOUBLEBUFFER;
	pfd.iLayerType = PFD_MAIN_PLANE;
	pfd.iPixelType = PFD_TYPE_RGBA;
	pfd.nSize = sizeof(PIXELFORMATDESCRIPTOR);
	pfd.nVersion = 1;

	// Get the closest match pixel format
	pixel_format = ChoosePixelFormat(hDC, &pfd);

	// Set the window's format to this
	if (SetPixelFormat(hDC, pixel_format, &pfd) == FALSE)
		return 0;

	// Create an OpenGL Rendering Context on top of the DC
	if ((hRC = wglCreateContext(hDC)) == NULL)
		return 0;

	// Make this the current rendering context
	if (wglMakeCurrent(hDC, hRC) == FALSE)
		return 0;

	return 1;
}


void EndOpenGL()
{
	// Release the current rendering context
	wglMakeCurrent(NULL, NULL);

	// Now delete it
	wglDeleteContext(hRC);
}


long WINAPI WndProc(HWND hWnd, UINT message, WPARAM wParam, LPARAM lParam)
{
	switch (message)
	{
		case (WM_CREATE):
			break;

		case (WM_CLOSE):
			DestroyWindow(hWnd);
			break;

		case (WM_DESTROY):
            ChangeDisplaySettings(NULL, 0);

            // Release the window's DC
            ReleaseDC(hWnd, hDC);

			PostQuitMessage(0);
			break;

		default:
			return (DefWindowProc(hWnd, message, wParam, lParam));
	}

	return (0L);
}


int CreateApplicationWindow(const char* title, int width, int height, int style)
{
    WNDCLASS wc;

    // Needed for OpenGL
    style |= (WS_CLIPCHILDREN | WS_CLIPSIBLINGS);

    // Get the current application's instance
    if ((hInstance = GetModuleHandle(NULL)) == NULL)
        return 0;

    // Create window class
    wc.cbClsExtra    = 0;
    wc.cbWndExtra    = 0;
    wc.hbrBackground = (HBRUSH)COLOR_WINDOW;
    wc.hCursor       = LoadCursor(NULL, IDC_ARROW);
    wc.hIcon         = LoadIcon(NULL, IDI_APPLICATION);
    wc.hInstance     = hInstance;
    wc.lpfnWndProc   = WndProc;
    wc.lpszClassName = title;
    wc.lpszMenuName  = NULL;
    wc.style         = CS_VREDRAW | CS_HREDRAW | CS_OWNDC;

    // Register class with windows
    RegisterClass(&wc);

    // Create the window
    hWnd = CreateWindowEx(0,
        title,
        title,
        style,
        CW_USEDEFAULT,
        CW_USEDEFAULT,
        width,
        height,
        NULL,
        NULL,
        hInstance,
        NULL);

    // Failed?
    if (hWnd == NULL)
        return 0;

    // Can be done here because of CS_OWNDC
    hDC = GetDC(hWnd);

    // Show it
    UpdateWindow(hWnd);
    ShowWindow(hWnd, SW_SHOW);

    return (TRUE);
}


int PollMessageQueue()
{
	MSG		msg;

	// Loop while there are messages
	while (PeekMessage(&msg, NULL, 0, 0, PM_NOREMOVE))
	{
		// Got a WM_QUIT?
		if (!GetMessage(&msg, NULL, 0, 0))
			return 0;

		// Send out the message
		TranslateMessage(&msg);
		DispatchMessage(&msg);
	}

	return 1;
}


int RunOpenGLLoop()
{
	if (CreateApplicationWindow("Remotery OpenGL Test", 512, 512, WS_OVERLAPPEDWINDOW) == 0)
		return 0;

	if (!InitOpenGL())
		return 0;
 
    rmt_BindOpenGL();

    while (PollMessageQueue())
    {
        rmt_BeginOpenGLSample(MainLoop);

        glClearColor(1, 0, 0, 1);
        glClear(GL_COLOR_BUFFER_BIT);

        glBegin(GL_QUADS);
            glColor3f(0, 1, 0);
            glVertex2f(-0.5f, -0.5f);
            glVertex2f( 0.5f, -0.5f);
            glVertex2f( 0.5f,  0.5f);
            glVertex2f(-0.5f,  0.5f);
        glEnd();

        SwapBuffers(hDC);

        rmt_EndOpenGLSample();

        Sleep(10);
    }

    rmt_UnbindOpenGL();

    EndOpenGL();

    return 1;
}


int main()
{
    rmtError error;
    Remotery* rmt;

    error = rmt_CreateGlobalInstance(&rmt);
    if (error != RMT_ERROR_NONE)
    {
        printf("Fail\n");
        return 1;
    }

    if (!RunOpenGLLoop())
        return 1;

    if (!RunOpenGLLoop())
        return 1;

    rmt_DestroyGlobalInstance(rmt);

    printf("OK\n");
    return 0;

Compiles with:

cl opengl.c ..\lib\Remotery.c /DRMT_USE_OPENGL=1