Fix deadlock with Windows Loader during execute_while_frozen

[praydog]

I noticed a freeze when calling safetyhook::create_inline, and I assume this is where the problem came from. Either that or it was one of my many calls to execute_while_frozen.

[ThirteenAG]

I've started using safetyhook::execute_while_frozen, and a small number of reports claims that the game process hangs right after startup, doesn't happen on every launch as far as I can tell. I'm still trying to gather more info on this, since I can't reproduce.

Another thing I wanted to ask, when I tried to hook d3d present without execute_while_frozen, it crashed inside the hooked present right away because calling original function caused a nullptr exception. Is it not possible to fill that one first, then insert a jump to a hook, eliminating the need of execute_while_frozen?

[cursey]

Another thing I wanted to ask, when I tried to hook d3d present without execute_while_frozen, it crashed inside the hooked present right away because calling original function caused a nullptr exception. Is it not possible to fill that one first, then insert a jump to a hook, eliminating the need of execute_while_frozen?

InlineHook already assigns the trampoline before freezing the threads. The intent behind freezing the threads is to fix the IP of any thread that may be actively executing instructions we intend on replacing in a safe manner.

Can you explain how you were using execute_while_frozen in more detail? In general I would prefer to not even expose this function publicly since you have to be very careful in what you do while using it.

[ThirteenAG]

InlineHook already assigns the trampoline before freezing the threads. The intent behind freezing the threads is to fix the IP of any thread that may be actively executing instructions we intend on replacing in a safe manner.

I was sent a memory dump, and apparently it's not execute_while_frozen issue, I called safetyhook::InlineHook::stdcall instead of unsafe_stdcall inside LdrLoadDll which resulted in this hang for some people:

 	ntdll.dll!_NtWaitForAlertByThreadId@8()	Unknown	Non-user code. Symbols loaded without source information.
 	ntdll.dll!RtlAcquireSRWLockExclusive()	Unknown	Non-user code. Symbols loaded without source information.
>	GTAIV.EFLC.FusionFix.asi!mtx_do_lock(_Mtx_internal_imp_t * mtx=0x6f57f290, const _timespec64 * target=0x00000000) Line 95	C++	Non-user code. Symbols loaded.
 	GTAIV.EFLC.FusionFix.asi!_Mtx_lock(_Mtx_internal_imp_t * mtx=0x6f57f290) Line 163	C++	Non-user code. Symbols loaded.
 	[Inline Frame] GTAIV.EFLC.FusionFix.asi!std::_Mutex_base::lock() Line 56	C++	Symbols loaded.
 	[Inline Frame] GTAIV.EFLC.FusionFix.asi!std::scoped_lock<std::recursive_mutex>::{ctor}(std::recursive_mutex &) Line 507	C++	Symbols loaded.
 	[Inline Frame] GTAIV.EFLC.FusionFix.asi!safetyhook::InlineHook::stdcall(wchar_t *) Line 388	C++	Symbols loaded.
 	GTAIV.EFLC.FusionFix.asi!LdrLoadDllHook(wchar_t * PathToFile=0x00000801, unsigned long * Flags=0x0019e9c8, _UNICODE_STRING * ModuleFileName=0x0019e9d8, HINSTANCE__ * * ModuleHandle=0x0019e9cc) Line 45	C++	Symbols loaded.
 	KERNELBASE.dll!LoadLibraryExW()	Unknown	Non-user code. Symbols loaded without source information.

Can you explain how you were using execute_while_frozen in more detail? In general I would prefer to not even expose this function publicly since you have to be very careful in what you do while using it.

Yes, I've replaced a code that utilized minhook with safetyhook, to inject into d3d device vtable calls:

    static inline bool bind(HMODULE module, std::type_index type_index, uint16_t func_index, void* function, void** original = nullptr)
    {
        auto target = deviceMethods.at(module).at(type_index).at(func_index);
        if (MH_CreateHook(target, function, original) != MH_OK || MH_EnableHook(target) != MH_OK) {
            return false;
        }
        return true;
    }

    static inline void bind(HMODULE module, std::type_index type_index, uint16_t func_index, void* function, SafetyHookInline& hook)
    {
        auto target = deviceMethods.at(module).at(type_index).at(func_index);
        try
        {
            safetyhook::execute_while_frozen([&]
            {
                hook = safetyhook::create_inline(target, function);
            });
        }
        catch (const std::exception& e) {}
    }

It doesn't work without execute_while_frozen, crashes.

Links to code mentioned above: https://github.com/ThirteenAG/FusionDxHook/blob/main/includes/fusiondxhook.h#L1093-L1104 https://github.com/ThirteenAG/GTAIV.EFLC.FusionFix/blob/master/source/dllblacklist.ixx#L28-L46

[cursey]

    static inline void bind(HMODULE module, std::type_index type_index, uint16_t func_index, void* function, SafetyHookInline& hook)
    {
        auto target = deviceMethods.at(module).at(type_index).at(func_index);
        try
        {
            safetyhook::execute_while_frozen([&]
            {
                hook = safetyhook::create_inline(target, function);
            });
        }
        catch (const std::exception& e) {}
    }

It doesn't work without execute_while_frozen, crashes.

I'm confused by this code. You shouldn't ever have to wrap safeythook::create_inline in execute_while_frozen. It uses execute_while_frozen internally already. Regardless, I think I've got an idea on how to improve the library while removing execute_while_frozen entirely since it seems to be a point of confusion and miss use. I'll have a PR up soon with my idea if it works out.

[angelfor3v3r]

execute_while_frozen is really easy to misuse. I don't think it was ever suggested to be used from the public API, and the inline hooks will already do it for you. There's even a note here, which warns against it: https://github.com/cursey/safetyhook/blob/3103d4b5964e0bc5b838b8e4807a9ba720901f0a/include/safetyhook/thread_freezer.hpp#L20

Unless you absolutely know you have to use it, then you shouldn't use it.

@ThirteenAG I guess we're more curious to why exactly you need to use it here. Could you elaborate on this or provide more details? Are you sure you need to use execute_while_frozen? I understand you said it crashes, but is it possible it's something else? Any more information really helps, thanks!

[ThirteenAG]

You shouldn't ever have to wrap safeythook::create_inline in execute_while_frozen. It uses execute_while_frozen internally already.

Yes, but it didn't work for me with Max Payne 3 d3d hooking and resulted in a crash. It doesn't happen with execute_while_frozen.

 	00000000()	Unknown	No symbols loaded.
 	[Frames below may be incorrect and/or missing]		Annotated Frame
>	[Inline Frame] MaxPayne3.XboxRainDroplets.asi!safetyhook::InlineHook::unsafe_stdcall(IDXGISwapChain *) Line 482	C++	Symbols loaded.
 	[Inline Frame] MaxPayne3.XboxRainDroplets.asi!FusionDxHook::HookD3D10_1::__l2::<lambda_2>::()::__l11::<lambda_1>::operator()(IDXGISwapChain *) Line 640	C++	Symbols loaded.
 	MaxPayne3.XboxRainDroplets.asi!``FusionDxHook::HookD3D10_1'::`2'::<lambda_2>::operator()'::`11'::<lambda_1>::<lambda_invoker_stdcall>(IDXGISwapChain * pSwapChain, unsigned int SyncInterval=0x00000000, unsigned int Flags=0x00000000) Line 680	C++	Symbols loaded.
 	MaxPayne3.exe!00f0f222()	Unknown	Non-user code. Cannot find or open the PDB file.

Crash on this line: https://github.com/ThirteenAG/FusionDxHook/blob/main/includes/fusiondxhook.h#L640 But do note I'm using dx11 renderer in the game, not 10.1. Also worked with minhook implementation.

That's literally all I have to do to trigger the crash immediately (commented lines):

    static inline void bind(HMODULE module, std::type_index type_index, uint16_t func_index, void* function, SafetyHookInline& hook)
    {
        auto target = deviceMethods.at(module).at(type_index).at(func_index);
        try
        {
            //safetyhook::execute_while_frozen([&]
            //{
                hook = safetyhook::create_inline(target, function);
            //});
        }
        catch (const std::exception& e) {}
    }

As I've said, works inside execute_while_frozen, no problems. I'd assume due to create_inline hooking, but hook is not assigned yet and crashes?

[cursey]

@ThirteenAG can you try this quick test PR I threw together? #63

You'll need to remove execute_while_frozen calls from your code to test it.

[ThirteenAG]

@ThirteenAG can you try this quick test PR I threw together? #63

You'll need to remove execute_while_frozen calls from your code to test it.

Tested GTAIV/Max Payne 3 plugins, and dx hook tests with all uses of execute_while_frozen removed, works fine.

[angelfor3v3r]

@ThirteenAG can you try this quick test PR I threw together? #63 You'll need to remove execute_while_frozen calls from your code to test it.

Tested GTAIV/Max Payne 3 plugins, and dx hook tests with all uses of execute_while_frozen removed, works fine.

Thanks for the extra information, it really helps. And thanks for testing it. I think this change is better in the long term since it's technically more portable too.

[cursey]

Closing this since I landed thread trapping.