TsudaKageyu / minhook

The Minimalistic x86/x64 API Hooking Library for Windows
http://www.codeproject.com/KB/winsdk/LibMinHook.aspx
Other
4.43k stars 897 forks source link

Mitigates #132 #134

Closed FransBouma closed 3 months ago

FransBouma commented 3 months ago

Changes to mitigate the issue where SuspendThread() returns -1 (error occurred) which would then crash the host because the ResumeThread() call in Unfreeze would crash.

FransBouma commented 3 months ago

Also, should there be a slight delay after Freeze()? SuspendThread isn't instant, it takes some time to have a thread become paused...

m417z commented 3 months ago

Thanks. Left several comments.

Also, should there be a slight delay after Freeze()? SuspendThread isn't instant, it takes some time to have a thread become paused...

Get­Thread­Context should take care of it, see: https://devblogs.microsoft.com/oldnewthing/20150205-00/?p=44743

FransBouma commented 3 months ago

Also, should there be a slight delay after Freeze()? SuspendThread isn't instant, it takes some time to have a thread become paused...

Get­Thread­Context should take care of it, see: https://devblogs.microsoft.com/oldnewthing/20150205-00/?p=44743

I recon this call should be before ProcessThreadIPs()? (In Freeze)

m417z commented 3 months ago

I recon this call should be before ProcessThreadIPs()? (In Freeze)

That's the first thing ProcessThreadIPs does with the thread.

FransBouma commented 3 months ago

Committed a new commit, with the requested changes. Not yet with a call to GetThreadContext() btw.

FransBouma commented 3 months ago

I recon this call should be before ProcessThreadIPs()? (In Freeze)

That's the first thing ProcessThreadIPs does with the thread.

Ah, so the Sleep() I added in my own build didn't add anything, as GetThreadContext already made sure the code waits for the thread to get suspended before returning. Hmm... Interesting.

m417z commented 3 months ago

Ah, so the Sleep() I added in my own build didn't add anything

Well, maybe it made the issue less likely.