Skip to content

Commit

Permalink
Fix race conditions in UiaTextRangeBase (microsoft#17257)
Browse files Browse the repository at this point in the history
We need to lock the buffer when getting the viewport/cursor position.
This caused the UIA overlay to randomly fail to update.

## Validation Steps Performed
* Open a cmd tab and hold any key immediately
* Repeat until you're somewhat confident it's gone ✅
  • Loading branch information
lhecker authored May 13, 2024
1 parent 46526bc commit e1b102a
Showing 1 changed file with 21 additions and 10 deletions.
31 changes: 21 additions & 10 deletions src/types/UiaTextRangeBase.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,15 @@ try
RETURN_HR_IF_NULL(E_INVALIDARG, pProvider);
RETURN_HR_IF_NULL(E_INVALIDARG, pData);

pData->LockConsole();
const auto unlock = wil::scope_exit([&]() noexcept {
pData->UnlockConsole();
});

_pProvider = pProvider;
_pData = pData;
_start = pData->GetViewport().Origin();
_end = pData->GetViewport().Origin();
_end = _start;
_blockRange = false;
_wordDelimiters = wordDelimiters;

Expand All @@ -41,16 +46,25 @@ HRESULT UiaTextRangeBase::RuntimeClassInitialize(_In_ Render::IRenderData* pData
_In_ std::wstring_view wordDelimiters) noexcept
try
{
RETURN_HR_IF_NULL(E_INVALIDARG, pProvider);
RETURN_HR_IF_NULL(E_INVALIDARG, pData);
RETURN_IF_FAILED(RuntimeClassInitialize(pData, pProvider, wordDelimiters));

pData->LockConsole();
const auto unlock = wil::scope_exit([&]() noexcept {
pData->UnlockConsole();
});

_pProvider = pProvider;
_pData = pData;
// GH#8730: The cursor position may be in a delayed state, resulting in it being out of bounds.
// If that's the case, clamp it to be within bounds.
// TODO GH#12440: We should be able to just check some fields off of the Cursor object,
// but Windows Terminal isn't updating those flags properly.
_start = cursor.GetPosition();
pData->GetTextBuffer().GetSize().Clamp(_start);
_end = _start;
_blockRange = false;
_wordDelimiters = wordDelimiters;

UiaTracing::TextRange::Constructor(*this);
return S_OK;
Expand All @@ -66,15 +80,15 @@ HRESULT UiaTextRangeBase::RuntimeClassInitialize(_In_ Render::IRenderData* pData
_In_ std::wstring_view wordDelimiters) noexcept
try
{
RETURN_IF_FAILED(RuntimeClassInitialize(pData, pProvider, wordDelimiters));
RETURN_HR_IF_NULL(E_INVALIDARG, pProvider);
RETURN_HR_IF_NULL(E_INVALIDARG, pData);

// start must be before or equal to end
_pProvider = pProvider;
_pData = pData;
_start = std::min(start, end);
_end = std::max(start, end);

// This should be the only way to set if we are a blockRange
// This is used for blockSelection
_blockRange = blockRange;
_wordDelimiters = wordDelimiters;

UiaTracing::TextRange::Constructor(*this);
return S_OK;
Expand Down Expand Up @@ -148,9 +162,6 @@ til::point UiaTextRangeBase::GetEndpoint(TextPatternRangeEndpoint endpoint) cons
// - true if range is degenerate, false otherwise.
bool UiaTextRangeBase::SetEndpoint(TextPatternRangeEndpoint endpoint, const til::point val) noexcept
{
// GH#6402: Get the actual buffer size here, instead of the one
// constrained by the virtual bottom.
const auto bufferSize = _pData->GetTextBuffer().GetSize();
switch (endpoint)
{
case TextPatternRangeEndpoint_End:
Expand Down

0 comments on commit e1b102a

Please sign in to comment.