discord / gamesdk-and-dispatch

Public issue tracker for the Discord Game SDK and Dispatch
22 stars 7 forks source link

discord::Core throws exception on deletion #37

Open qaisjp opened 4 years ago

qaisjp commented 4 years ago

Describe the bug

Where discord::Core* m_DiscordCore, running delete m_DiscordCore will cause a Windows exception:

Unhandled exception at <offset> in <exe>: 0xC0000008: An invalid handle was specified. occurred

Steps to reproduce

if (!m_DiscordCore)
    return;

discord::Result res = m_DiscordCore->RunCallbacks();
if (res != discord::Result::Ok)
{
    delete m_DiscordCore;
    m_DiscordCore = nullptr;
}
  1. Start Discord
  2. Run a game with a loop containing the above code
  3. Stop Discord
  4. Game will error on line delete m_DiscordCore, but only if a debugger is attached

Expected behavior

No exception should be thrown.

Screenshots

image image

Implementation specifics

C++, Windows

Additional context

We delete the discord core, non-blocking wait 15s, and recreate the core. This handles the scenario where the user restarts discord whilst our game is running.

Someone on the discord-gamesdk guild said we shouldn't delee the core, but we keep getting discord::Result::NotRunning from RunCallbacks, despite the user starting Discord again. It seems the only way to get the game sdk to connect to the new instance of discord is to recreate discord::Core?

Is that a different bug?

ntdll.dll!_NtClose@4()
KernelBase.dll!CloseHandle()
discord_game_sdk.dll!__ZN62_$LT$miow..handle..Handle$u20$as$u20$core..ops..drop..Drop$GT$4drop17h0c181cb79ae10b12E()
discord_game_sdk.dll!__ZN4core3ptr18real_drop_in_place17h267655a0b744d984E.llvm.9039256649475794780()
discord_game_sdk.dll!__ZN4core3ptr18real_drop_in_place17h6a699bb6288f672cE.llvm.9039256649475794780()
discord_game_sdk.dll!__ZN64_$LT$alloc..rc..Rc$LT$T$GT$$u20$as$u20$core..ops..drop..Drop$GT$4drop17h70f0eaa83916689dE()
discord_game_sdk.dll!__ZN16discord_game_sdk4core8IDiscord3new17h56757ca96f5de7ddE()
core_d.dll!discord::Core::~Core() Line 46
    at E:\Documents\mtasa-blue\vendor\discordgsdk\cpp\core.cpp(46)
[External Code]
core_d.dll!CDiscordManager::DoPulse() Line 212
    at E:\Documents\mtasa-blue\Client\core\CDiscordManager.cpp(212)
sylveon commented 4 years ago

Seeing the same issue in an standalone C++ app. I want my app to be able to handle discord closing and restarting, so I need to recreate the core as well.

sylveon commented 4 years ago

I use mitigation policies to disallow invalid handles as well, so the issue is present even without a debugger.

qaisjp commented 4 years ago

If only the library was open source and we could submit a pull request fixing this 🙃

sylveon commented 4 years ago

Here's a minimum, 100% reliable repro (on Windows):

#include <iostream>
#include <discord-game-sdk/discord.h>
#include <Windows.h>

int main()
{
    PROCESS_MITIGATION_STRICT_HANDLE_CHECK_POLICY handle_policy{};
    handle_policy.RaiseExceptionOnInvalidHandleReference = true;
    handle_policy.HandleExceptionsPermanentlyEnabled = true;
    SetProcessMitigationPolicy(ProcessStrictHandleCheckPolicy, &handle_policy, sizeof(handle_policy));

    discord::Core* core{};
    auto response = discord::Core::Create(461618159171141643, DiscordCreateFlags_Default, &core);

    if (!core) {
        std::cout << "Failed to instantiate Discord! " << (int)response;
        std::exit(-1);
    }

    delete core; // crashes here for me
}

@msciotti can you please take a look, or forward this to someone who can?

sylveon commented 4 years ago

The only solution currently is intentionally leaking the core :(

Please fix this

Kriper1111 commented 4 years ago

For some reason, calling the destructor explicitly core->~Core(); doesn't raise the error, while delete core; does. (at least for me it didn't)

Mushe94 commented 3 years ago

A similar error (InternalError) is thrown if you open your game without having Discord opened. And since there is no way of detecting if Discord is opened mid session there is no way on using the SDK in a proper way.

I mean you can try like you said every X amount of seconds to create a new Discord instance to see if "hey, did you opened this time?" but really that's extremely inefficient (specially if the user literally never opens Discord, it's going to be trying to recreate something that the player never intends to do).

sylveon commented 3 years ago

InternalError isn't even remotely similar. The exception here is triggered by Windows because the SDK uses an invalid handle and completely and irrecoverably crashes your program, while InternalError is recoverable and only thrown in C# (it's a return value in C++)

emilykmcullen commented 3 years ago

Can confirm this issue is still happening, will this be fixed anytime soon?