microsoft / terminal

The new Windows Terminal and the original Windows console host, all in the same place!
MIT License
95k stars 8.22k forks source link

Lock and Unlock in conhost should decouple Ctrl+C dispatch and use smarter handling #2141

Open miniksa opened 5 years ago

miniksa commented 5 years ago

The Lock and Unlock procedures in Conhost.exe are fraught with error.

For one, we are using intricate details of how many recursive entries there are on the lock to count when it is fully unlocked and dispatch Ctrl+C events. We're not supposed to be dependent on the lock count at all.

Additionally, there's multiple layers at which the locks can be processed, some of which DO dispatch the Ctrl+C events on the last unlock and some of which do NOT.

Finally, the lock/unlock procedure is bad in that it is easy to not unlock something since it's not in any sort of smart RAII-type object. This is sort of mitigated by wil::scope_exit in many places, but that function is also discouraged when you can use something better.

This issue encompasses rooting around and finding a better overall way to do all of this.

It does NOT encompass more granular locks than already exist.

zadjii-msft commented 5 years ago

where's the "crying inside" reaction when you need it

miniksa commented 5 years ago

🤪

DHowett-MSFT commented 5 years ago

Yanking triage ;P