[deadlock fix] Remove lock for SIUP::GetSelectionRange() #11386
Conversation
carlos-zamora
added
zStable-Service-Queued-1.12
ghost
added
Area-Accessibility
| @@ -849,7 +849,13 @@ WORD Terminal::_TakeVirtualKeyFromLastKeyEvent(const WORD scanCode) noexcept | |||
| // will release this lock when it's destructed. | |||
| [[nodiscard]] std::unique_lock<til::ticket_lock> Terminal::LockForReading() | |||
| { | |||
| #ifdef NDEBUG | |||
There was a problem hiding this comment.
I've always done ifdef DBG and done it the other way. @lhecker would know if this is fine.
carlos-zamora
added
AutoMerge
|
Hello @carlos-zamora! Because this pull request has the Do note that I've been instructed to only help merge pull requests of this repository that have been opened for at least 8 hours, a condition that will be fulfilled in about 7 hours 32 minutes. No worries though, I will be back when the time is right! 😉 p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
|
@msftbot merge this in 1 minute |
|
Hello @carlos-zamora! Because you've given me some instructions on how to help merge this pull request, I'll be modifying my merge approach. Here's how I understand your requirements for merging this pull request:
If this doesn't seem right to you, you can tell me to cancel these instructions and use the auto-merge policy that has been configured for this repository. Try telling me "forget everything I just told you". |
ghost
merged commit ## Summary of the Pull Request The deadlock was caused by `ScreenInfoUiaProviderBase::GetSelection()` calling `TermControlUiaProvider::GetSelectionRange` (both of which attempted to lock the console). This PR removes the lock and initialization check from `TermControlUiaProvider`. It is no longer necessary because the only one that calls it is `SIUPB::GetSelection()`. Additionally, this adds some code that was useful in debugging this race condition. That should help us figure out any locking issues that may come up in the future. ## References #11312 Closes #11385 ## Validation Steps Performed ✅ Repro steps don't cause hang
|
🎉 Handy links: |
ghost
mentioned this pull request
DHowett
removed
the
zPreview-Service-Queued-1.13
|
🎉 Handy links: |
|
🎉 Handy links: |
Summary of the Pull Request
The deadlock was caused by
ScreenInfoUiaProviderBase::GetSelection()callingTermControlUiaProvider::GetSelectionRange(both of which attempted to lock the console). This PR removes the lock and initialization check fromTermControlUiaProvider. It is no longer necessary because the only one that calls it isSIUPB::GetSelection().Additionally, this adds some code that was useful in debugging this race condition. That should help us figure out any locking issues that may come up in the future.
References
#11312
Closes #11385
Validation Steps Performed
✅ Repro steps don't cause hang