1

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.

YoonSeok OH
  • 647
  • 2
  • 7
  • 15
  • What is the exact message for `C28112` not just some code number ? – Richard Critten Sep 25 '21 at 14:10
  • 1
    Does this answer your question? [Atomic load in C with MSVC](https://stackoverflow.com/questions/42660091/atomic-load-in-c-with-msvc) – Richard Critten Sep 25 '21 at 14:11
  • 1
    @RichardCritten `A variable which is accessed via an Interlocked function must always be accessed via an Interlocked function` – WBuck Sep 25 '21 at 14:13
  • Thanks for the comment Richard Critten, message of C28112 is "warning C28112: A variable which is accessed via an Interlocked function must always be accessed via an Interlocked function" – YoonSeok OH Sep 25 '21 at 14:32
  • 2
    Putting the variable in a struct avoids the warning, like in here (OwnerThread is in a struct ref): https://devblogs.microsoft.com/oldnewthing/20160825-00/?p=94165 – Simon Mourier Sep 25 '21 at 14:41
  • Thanks again Richard Critten, https://stackoverflow.com/questions/42660091/atomic-load-in-c-with-msvc According to this question, it seems there are 3 options which are 1) disable warning, 2) Use InterlockedOr, and 3) Use InterlockedCompareExchange. Yes it answers my question. BTW it is quite surprising that there is no interlocked read only function. – YoonSeok OH Sep 25 '21 at 14:41
  • Thanks for the comment Simon Mourier. Putting variable in a struct avoids warning? I must check that up right now. – YoonSeok OH Sep 25 '21 at 14:45
  • this warning wrong by design at all. not all access must be interlocked. but only RMW (read-modify-write) access. when we do only read or only write - no sense in lock here. must be only atomic. no sense in `InterlockedOr16(&g_sShared, 0i16) == 1i16` check, `g_sShared == 1` is ok and enough if it atomic (if correct aligned - atomic). – RbMm Sep 25 '21 at 19:50

1 Answers1

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.

Alex Guteniev
  • 12,039
  • 2
  • 34
  • 79