Unloading DLL without manually resetting hooks no longer restores original bytes

[aixxe]

Recently ran into an issue with v0.5.x where hooking a function from a DLL, then unloading it will cause any subsequent calls to the function to crash. Destructing the hook manually in the DllMain detach notification works, but this wasn't necessary in v0.4.1.

Example that builds against every amalgamated version from v0.4.1 onwards: test.zip

Stack trace below is from Clang-cl 18.1.8, same as with MSVC 17.12.3. This issue does not occur with MSYS2 (UCRT64) GCC 14.2.0.

[hook-0.5.2.dll] std::_Tree::_Find_lower_bound<…>(unsigned char *const &) xtree:1620
[hook-0.5.2.dll] safetyhook::TrapManager::add_trap(unsigned char *, unsigned char *, unsigned long long) safetyhook.cpp:1427
[hook-0.5.2.dll] safetyhook::trap_threads(unsigned char *, unsigned char *, unsigned long long, const std::function<…> &) safetyhook.cpp:1496
[hook-0.5.2.dll] safetyhook::InlineHook::disable() safetyhook.cpp:818
[hook-0.5.2.dll] safetyhook::InlineHook::destroy() safetyhook.cpp:827
[hook-0.5.2.dll] safetyhook::InlineHook::~InlineHook() safetyhook.cpp:566
[hook-0.5.2.dll] `dynamic atexit destructor for 'hook'() hook.cpp:4
[host-0.5.2.exe] main() host.cpp:14

This doesn't quite tell the whole story though, as it only seems to crash in this way when running with a debugger attached. When run without, it unloads the hook library, makes it to the pause call, then crashes when running the previously hooked print function.

I checked where the hook was placed and saw the jmp was still there, even after the DLL had been unloaded.

The convenience of not having to worry about manually unhooking was nice, but now that I'm looking at the example code again, I realize I might've been getting away with misusing it until now. I thought it would be worth mentioning just to be sure.

[cursey]

I think we can support easily unhooking everything. DllMain gets called when unloading so sticking a call to cleanup everything there seems easy enough, or having a global object who's destructor cleans everything up. I have to give it some thought. Thanks for the report.

[cursey]

A minimal reproducible test for this failing behavior would be nice if you have the time.

[aixxe]

Sorry for the late reply. There's an example project in the first post that should show the crash in the host-0.5.0 and later targets. The working output should be something like this:

> .\cmake-build-debug\bin\0.4.1\host-0.4.1.exe
print: before installing hook
print: hooked!
Press any key to continue . . .
print: after uninstalling hook

Process finished with exit code 0

For later targets, it should crash before printing the final line:

.\cmake-build-debug\bin\0.5.0\host-0.5.0.exe
print: before installing hook
print: hooked!
Press any key to continue . . .

Process finished with exit code -1073741819 (0xC0000005)

---

From playing around a bit more, it seems like TrapManager has already been destructed by the time the hooks are disabled. Here's the same output with a few print lines added to the destructor and before/after the insert_or_assign in add_trap

print: before installing hook
[TrapManager::instance->add_trap] m_traps.size=0
[/TrapManager::instance->add_trap] m_traps.size=1
print: hooked!
[TrapManager::~TrapManager]
[TrapManager::instance->add_trap] m_traps.size=15987178197214944733
Press any key to continue . . .

Process finished with exit code -1073741819 (0xC0000005)

[cursey]

I'm still holiday brained and I'm not in love with this approach but as a fast and dirty solution it works. Can you give #88 a try and let me know if it fixes the DLL unloading problem in your real projects, not just the test one.

Thanks!

[ThirteenAG]

I noticed that Splinter Cell Conviction shows an error message when exiting with enabled hooks(didn't before thread trapping was implemented), so I tested on that one. I reproduced the message with current code, then replaced the file in the PR. The message is gone, but seemingly I'm having the game stuck in a loop on exit still:

EDI content:

And console log just spams:

Exception thrown at 0x60F63AB9 (SplinterCellConviction.FusionMod.asi) in conviction_game.exe: 0xC0000005: Access violation writing location 0x162F1BEF.
Exception thrown at 0x60F63AB9 (SplinterCellConviction.FusionMod.asi) in conviction_game.exe: 0xC0000005: Access violation writing location 0x162F1BEF.
Exception thrown at 0x60F63AB9 (SplinterCellConviction.FusionMod.asi) in conviction_game.exe: 0xC0000005: Access violation writing location 0x162F1BEF.
Exception thrown at 0x60F63AB9 (SplinterCellConviction.FusionMod.asi) in conviction_game.exe: 0xC0000005: Access violation writing location 0x162F1BEF.
...
Exception thrown at 0x60F63AB9 (SplinterCellConviction.FusionMod.asi) in conviction_game.exe: 0xC0000005: Access violation writing location 0x162F1BEF.
Exception thrown at 0x60F63AB9 (SplinterCellConviction.FusionMod.asi) in conviction_game.exe: 0xC0000005: Access violation writing location 0x162F1BEF.