Improve multi-threaded code design to prevent race condition

Question

I'm running into an issue that I'm not sure is solvable in the way I want to solve it. I have a problem with a race condition.

I have one project running as a C++ dll (the main engine). Then I have a second C# process that uses C++/CLI to communicate with the main engine (the editor).

The editor is hosting the engine window as a child window. The result of this is that the child window receives input messages async (see RiProcessMouseMessage()). Normally this only happens when I call window->PollEvents();.

main engine loop {
    RiProcessMouseMessage(); // <- Called by the default windows message poll function from the child window
    
    foreach(inputDevice)
        inputDevice->UpdateState();

    otherCode->UseCurrentInput();
}

The main editor loop is the WPF loop which I don't control. Basically it does this:

main editor loop {
    RiProcessMouseMessage(); // <- This one is called by the editor (parent) window, but is using the message loop of the (child) engine window
}

The RawInput processor which is called sync by the engine and async by the editor:

void Win32RawInput::RiProcessMouseMessage(const RAWMOUSE& rmouse, HWND hWnd) {
    MouseState& state = Input::mouse._GetGatherState();

    // Check Mouse Position Relative Motion
    if (rmouse.usFlags == MOUSE_MOVE_RELATIVE) {
        vec2f delta((float)rmouse.lLastX, (float)rmouse.lLastY);
        delta *= MOUSE_SCALE;
        state.movement += delta;

        POINT p;
        GetCursorPos(&p);
        state.cursorPosGlobal = vec2i(p.x, p.y);

        ScreenToClient(hWnd, &p);
        state.cursorPos = vec2i(p.x, p.y);
    }

    // Check Mouse Wheel Relative Motion
    if (rmouse.usButtonFlags & RI_MOUSE_WHEEL)
        state.scrollMovement.y += ((float)(short)rmouse.usButtonData) / WHEEL_DELTA;

    if (rmouse.usButtonFlags & RI_MOUSE_HWHEEL)
        state.scrollMovement.x += ((float)(short)rmouse.usButtonData) / WHEEL_DELTA;

    // Store Mouse Button States
    for (int i = 0; i < 5; i++) {
        if (rmouse.usButtonFlags & maskDown_[i]) {
            state.mouseButtonState[i].pressed = true;
            state.mouseButtonState[i].changedThisFrame = true;
        } else if (rmouse.usButtonFlags & maskUp_[i]) {
            state.mouseButtonState[i].pressed = false;
            state.mouseButtonState[i].changedThisFrame = true;
        }
    }
}

UpdateState() is called only by the engine. It basically swaps the RawInput to the currently used input. This is to prevent input updating in the middle of a frame loop (aka. during otherCode->UseCurrentInput();)

void UpdateState() {
    currentState = gatherState; // Copy gather state to current
    Reset(gatherState);         // Reset the old buffer so the next time the buffer it's used it's all good
    
   // Use current state to check stuff
   // For the rest of this frame currentState should be used
}

MouseState& _GetGatherState() { return gatherState; }

void Reset(MouseState& state) { // Might need a lock around gatherState :(
    state.movement = vec2f::zero;
    state.scrollMovement = vec2f::zero;

    for (int i = 0; i < 5; ++i)
        state.mouseButtonState[i].changedThisFrame = false;
}

So as you can see the race condition happens when RiProcessMouseMessage() is called while Reset() was called in the main engine loop. If it wasn't clear: The Reset() function is required to reset state back to it's frames default data so that the data is read correctly every frame.

Now I'm very much aware I can fix this easily by adding a mutex around the gatherState updates but I would like to avoid this if possible. Basically I'm asking is it possible to redesign this code to be lock free?

Abdurrahim · Answer 1 · 2020-12-23T21:39:31.380

You are asking lock-free which is not quite possible if both ends alter the buffer. But if you ask lock that is optimized and almost instantaneous then you can use FIFO logic. You can use the .net's ConcurrentQueue "https://learn.microsoft.com/en-us/dotnet/api/system.collections.concurrent.concurrentqueue-1?view=net-5.0" to write updates and poll updates from this queue.

If you really get rid of the lock then you may check lock-free circular arrays aka lock-free ring-buffer, If you want to dig deeper into hardware level to understand the logic behind this then you can check https://electronics.stackexchange.com/questions/317415/how-to-allow-thread-and-interrupt-safe-writing-of-incoming-usart-data-on-freerto so you will have an idea about concurrency at the low-level as well; With limitations, a lock-free ring buffer can work when one end only writes and the other end only reads within known intervals/boundaries can check similar questions asked: Circular lock-free buffer

Boost has well-known implementations for lock-free: https://www.boost.org/doc/libs/1_65_1/doc/html/lockfree.html

Hmm,, yes a queue might actually work here. I actually need it in C++, but I already have a concurrent queue implemenation laying around. I'll give it a go tomorrow. Thanks for the suggestion, sometimes I just need a second pair of eyes to look at a problem. — Duckdoom5, Dec 23 '20 at 21:47

Improve multi-threaded code design to prevent race condition

1 Answers1