clink icon indicating copy to clipboard operation
clink copied to clipboard

Speed up clink injection by not suspending threads

Open jalfd opened this issue 6 years ago • 2 comments

Iterating over every thread on the system takes a lot of time, and is unnecessary as cmd.exe is already blocked (waiting in WaitForSingleObject()) whenever clink is being injected. So just don't do it.

Also add a few static_cast's to silence warnings in VS2017 about narrowing conversions.

jalfd avatar Jan 14 '19 08:01 jalfd

is unnecessary as cmd.exe is already blocked (waiting in WaitForSingleObject()) whenever clink is being injected

That's only true when clink is being injected into the same cmd.exe that spawned the clink inject command. The --pid flag can inject into any cmd.exe. Also it can inject into a grandparent cmd.exe -- for example cmd.exe spawns devenv.exe and then devenv.exe spawns clink_x64.exe inject and then you run dir /s c:\ in the grandparent cmd.exe process: now hit F5 in the debugger to start the injection. Cmd.exe is not already blocked waiting.

This change sounds like it may accidentally remove support for less common ways that clink can be injected. Perhaps there could be a flag so that it's not the default way, but can be forced if the user is willing to take the risk.

chrisant996 avatar Sep 13 '20 03:09 chrisant996

is unnecessary as cmd.exe is already blocked (waiting in WaitForSingleObject()) whenever clink is being injected

That's only true when clink is being injected into the same cmd.exe that spawned the clink inject command. The --pid flag can inject into any cmd.exe. Also it can inject into a grandparent cmd.exe -- for example cmd.exe spawns devenv.exe and then devenv.exe spawns clink_x64.exe inject and then you run dir /s c:\ in the grandparent cmd.exe process: now hit F5 in the debugger to start the injection. Cmd.exe is not already blocked waiting.

This change sounds like it may accidentally remove support for less common ways that clink can be injected. Perhaps there could be a flag so that it's not the default way, but can be forced if the user is willing to take the risk.

That's true. I hadn't considered that use case (because I don't use it). Thanks for spotting that!

jalfd avatar Nov 24 '20 11:11 jalfd