How to use interlocked variable for WaitOnAddress / WakeByAddressSingle?

Question

Recently I've got to know WaitOnAddress, WakeByAddressSingle, and Interlocked* family of functions. While making test code as a part of understanding process, I've faced C28112 warning.

Following code is the test code that generates C28112 warning.

#include <iostream>
#include <Windows.h>
#pragma comment(lib, "Synchronization.lib")

using namespace std;

void* g_pThread0 = nullptr;
unsigned long g_ulThread0ID = 0UL;

void* g_pThread1 = nullptr;
unsigned long g_ulThread1ID = 0UL;

short g_sShared = 0i16;
short g_sCompare = 0i16;

unsigned long __stdcall Thread0(void* const _pParameter)
{
    int nWaitResult = -1;
    short sExchangeResult = -1i16;

    while (true)
    {
        nWaitResult = WaitOnAddress(&g_sShared, &g_sCompare, sizeof(short), INFINITE);  // C28112
        if (InterlockedOr16(&g_sShared, 0i16) == 1i16)   // Check
        {
            // Do Something

            sExchangeResult = InterlockedExchange16(&g_sShared, 0i16);
        }
    }

    return 0UL;
}

unsigned long __stdcall Thread1(void* const _pParameter)
{
    short sExchangeResult = -1i16;

    while (true)
    {
        sExchangeResult = InterlockedExchange16(&g_sShared, 1i16);
        WakeByAddressSingle(&g_sShared);    // C28112
    }

    return 0UL;
}

int main()
{
    g_pThread0 = CreateThread(nullptr, 0UL, Thread0, nullptr, 0UL, &g_ulThread0ID);
    g_pThread1 = CreateThread(nullptr, 0UL, Thread1, nullptr, 0UL, &g_ulThread1ID);

    while (true)
    {
    }

    return 0;
}

The following two lines are "accessing interlocked variable", I understand that.

nWaitResult = WaitOnAddress(&g_sShared, &g_sCompare, sizeof(short), INFINITE);  // C28112
...
WakeByAddressSingle(&g_sShared);    // C28112

The question is, how can I remove this warning while using interlocked variable for WaitOnAddress / WakeByAddressSingle?

Currently, I've come up with an answer by declaring g_sShared as pointer.

#include <iostream>
#include <Windows.h>
#pragma comment(lib, "Synchronization.lib")

using namespace std;

void* g_pThread0 = nullptr;
unsigned long g_ulThread0ID = 0UL;

void* g_pThread1 = nullptr;
unsigned long g_ulThread1ID = 0UL;

short* const g_pShared = new short(0i16);
short g_sCompare = 0i16;

unsigned long __stdcall Thread0(void* const _pParameter)
{
    int nWaitResult = -1;
    short sExchangeResult = -1i16;

    while (true)
    {
        nWaitResult = WaitOnAddress(g_pShared, &g_sCompare, sizeof(short), INFINITE);
        if (InterlockedOr16(g_pShared, 0i16) == 1i16)   // Check
        {
            // Do Something

            sExchangeResult = InterlockedExchange16(g_pShared, 0i16);
        }
    }

    return 0UL;
}

unsigned long __stdcall Thread1(void* const _pParameter)
{
    short sExchangeResult = -1i16;

    while (true)
    {
        sExchangeResult = InterlockedExchange16(g_pShared, 1i16);
        WakeByAddressSingle(g_pShared);
    }

    return 0UL;
}

int main()
{
    g_pThread0 = CreateThread(nullptr, 0UL, Thread0, nullptr, 0UL, &g_ulThread0ID);
    g_pThread1 = CreateThread(nullptr, 0UL, Thread1, nullptr, 0UL, &g_ulThread1ID);

    while (true)
    {
    }

    return 0;
}

This successfully removes warning. However, I feel this approach is a kind of warning removing trick.

[EDIT]

Thanks to the comments by Richard Critten, WBuck, Simon Mourier,

It seems there are 4 options to solve C28112 warning.

1. Disable warning.
2. Use InterlockedOr.
3. Use InterlockedCompareExchange.
4. Use struct wrapper for the interlocked variable.

1 and 4 seem to be the measures that solve the problem by bypassing warning. 2 and 3 seem to be the measures that solve the problem by satisfying warning .

Even though the warning is "overly cautious", 2 and 3 seem to be essentially problem solving way.

Following is the test code after using InterlockedOr.

#include <iostream>
#include <Windows.h>
#pragma comment(lib, "Synchronization.lib")

using namespace std;

void* g_pThread0 = nullptr;
unsigned long g_ulThread0ID = 0UL;

void* g_pThread1 = nullptr;
unsigned long g_ulThread1ID = 0UL;

short* g_pShared = new short(0i16);
short g_sCompare = 0i16;

unsigned long __stdcall Thread0(void* const _pParameter)
{
    int nWaitResult = -1;
    short sExchangeResult = -1i16;

    while (true)
    {
        nWaitResult = WaitOnAddress(reinterpret_cast<void*>(InterlockedOr64(reinterpret_cast<long long*>(&g_pShared), 0LL)), &g_sCompare, sizeof(short), INFINITE);
        if (InterlockedOr16(g_pShared, 0i16) == 1i16)   // Check
        {
            // Do Something

            sExchangeResult = InterlockedExchange16(g_pShared, 0i16);
        }
    }

    return 0UL;
}

unsigned long __stdcall Thread1(void* const _pParameter)
{
    short sExchangeResult = -1i16;

    while (true)
    {
        sExchangeResult = InterlockedExchange16(g_pShared, 1i16);
        WakeByAddressSingle(reinterpret_cast<void*>(InterlockedOr64(reinterpret_cast<long long*>(&g_pShared), 0LL)));
    }

    return 0UL;
}

int main()
{
    g_pThread0 = CreateThread(nullptr, 0UL, Thread0, nullptr, 0UL, &g_ulThread0ID);
    g_pThread1 = CreateThread(nullptr, 0UL, Thread1, nullptr, 0UL, &g_ulThread1ID);

    while (true)
    {
    }

    return 0;
}

Code seem surprisingly ugly. Any gentle person help me out please.

Answer 1

I think the warning here is false positive. It should make sure the variable is always accessed atomically, but WaitOnAddress and WakeByAddress* access the variable atomically internally.

You can suppress it using #pragma warning(supress:28112) above the line, or disable temporarily and then restore:

#pragma warning(push)
#pragma warning(disable:28112)
WakeByAddressSingle(...)
#pragma warning(pop)

You can report this false positive to Microsoft if you care (to https://developercommunity.visualstudio.com I guess)

---

Also if you are using Visual Studio 2019, more convenient way to have atomic wait is to use C++20 std::atomic<T>::wait / std::atomic<T>::notify_*. std::atomic should not produce this warning, and doesn't allow easy access to the underlying variable to access to it in non-atomic way.