-1

I have an input.hpp (which I won't post for the sake of brevity) and an input.cpp file that looks like this (some things removed):

#include "details/macros.hpp"
#if defined(PLATFORM_WINDOWS)
    #include "details/win32/input.inl"
#else
    #error "No input implementation for this platform."
#endif

#define CHECK_INPUT_TYPE(type)      \
if (types[input_type_bits::type])   \
{                                   \
    auto res = poll_##type();       \
    if (res.code() != errors::ok)   \
        LOG(error) << res;          \
}                                   

namespace input
{
        void poll_input(flagset<input_type_bits> types)
        {
            CHECK_INPUT_TYPE(keyboard)
            CHECK_INPUT_TYPE(mouse)
            CHECK_INPUT_TYPE(touch)
            CHECK_INPUT_TYPE(gesture)
            CHECK_INPUT_TYPE(gamepad)
        }
}

And an input.inl that looks like this (also cut down for brevity):

#ifndef WIN32_INPUT
#define WIN32_INPUT

#include <Windows.h>

namespace input
{
        static bool g_init = false;
        static std::shared_ptr<system::surface> g_surface = nullptr;

        static error<errors> poll_gamepad()
        {
            if (!g_init)
            {
                auto ptr = create_surface();
                g_surface = std::move(ptr);
                g_init = true;
            }

            HWND hwnd = reinterpret_cast<HWND>(g_surface->native_ptr());

            return MAKE_ERROR(errors::ok);
        }
}

#endif

However what is currently happening is that when I try to access g_surface's method, it works for the first call (in which g_init was false) but in the second time the poll_input function is called, according to Visual Studio's debugger, g_surface is empty and accessing the method throws an exception.
What gives? g_init was set to true successfully across calls and yet g_surface wasnt? If I move g_surface to be a static variable inside poll_gamepad it does work properly but that is something I'd like to avoid. Is there something about a static shared_ptr that I'm missing?

Alexandre Deus
  • 99
  • 1
  • 6
  • 1
    Every source file that includes `input.inl` will get its own private copies of the static variables defined in that file. This is quite the opposite of the "global" concept. Surely that is not what you intended. – paddy Jul 05 '22 at 23:58
  • 1
    Suggestion: wrap `g_surface` up in a function kind of like like a [Meyers singleton](https://stackoverflow.com/a/1008289/4581301). `std::shared_ptr & get_surface() { static std::shared_ptr instance = create_surface(); return instance; }`. This eliminates the need for `g_init` and makes the initialization threadsafe. Probably eliminates the need for a `shared_ptr`, too. It's now scoped by the function and will remain alive until after `main` exits. – user4581301 Jul 06 '22 at 00:04
  • @paddy `input.inl` is only included in `input.cpp` throughout the entire codebase, and the other parts of the codebase only have access to `poll_gamepad` through `poll_input` which is why I don't know what is causing my issue. @user4581301 That is indeed a typo. – Alexandre Deus Jul 06 '22 at 00:10
  • Recommendation: Make a [mre]. At the very least it'll reduce time wasted while we play whack-a-mole with the low-hanging fruit. – user4581301 Jul 06 '22 at 00:13
  • @user4581301 Unfortunately this is not a straightforward codebase to isolate. I already had to remove a lot of code just to give the examples. I will see what I can do. – Alexandre Deus Jul 06 '22 at 00:20
  • If you're 100% sure about this code only belonging to a single translation unit, then the next thing to consider is whether the first call to `poll_input` might be called simultaneously from more than one thread (the init has a race condition). However, that's a pretty "out there" theory without being able to see how `create_surface` behaves in such a scenario. Another possibility is you have a memory-smashing bug somewhere (perhaps static buffer overrun), or that somewhere else in `input.cpp` can legitimately reset `g_surface`. Do a "find all" on that identifier and check all usage. – paddy Jul 06 '22 at 01:26
  • To quickly check for memory-smashing, try adding static variables both before and after the shared_ptr definition, initialized with some kind of obvious bit pattern (e.g. `static volatile uint64_t mem_before = 0xa5a5a5a5a5a5a5a5;`) – paddy Jul 06 '22 at 01:31
  • @paddy I tried the before and after variable trick but the variables seemed to be untouched. I noticed there was a problem with another static variable I had though. An std::array of std::functions which on atexit would have a memory read exception in one of the std::function's destructor. I changed it to use function pointers instead which was a bandaid fix and I never really checked what was causing it. Is there a sure way to debug static memory issues that you can recommend? If it adds something of value, the code is in a shared library and it's being called in an executable. Thanks. – Alexandre Deus Jul 06 '22 at 10:44
  • Many compilers (unfortunately not as many on Windows) build in "sanitizers" to help you find hard-to-spot errors. |On GCC and clang, see if adding `-fsanitize=address,undefined` is supported by your tools. The extra checking will slow the program, but if it finds stuff, it's totally worth it . Remember to remove the option and recompile before you ship. – user4581301 Jul 06 '22 at 15:38
  • @user4581301 I've tried building with MSYS2's clang and aside from spewing a few exceptions from ntdll.dll which I googled and found them to be expected, nothing else showed up as being a problem. I'm completely stumped as to why I have static memory corruption as a problem in the entire codebase now. – Alexandre Deus Jul 06 '22 at 18:01
  • Please clarify your specific problem or provide additional details to highlight exactly what you need. As it's currently written, it's hard to tell exactly what you're asking. – Community Jul 17 '22 at 09:01

1 Answers1

0

I found out the answer to this problem. I mistakingly (don't code while tired people!) had a call to the Win32 API GetKeyboardState that was using the wrong static variable as the output buffer and caused static memory corruption.

Thank you for everyone's help and I apologize for not giving much information to deal with. That was my bad!

Anyways, I'm glad it's working properly now, case closed.

Alexandre Deus
  • 99
  • 1
  • 6