Prevent deadlock in UIA Move API (#10937)
Fixes a bug where interacting with Windows Terminal when using Narrator causes Windows Terminal to hang. `UiaTextRangeBase::Move()` locks, but later calls `UiaTextRangeBase::ExpandToEnclosingUnit()` which attempts to lock again. The workaround for this is to introduce a `_expandToEnclosingUnit()` that _does not_ lock the console. Then, `Move()` calls this new method, thus only allowing one lock to be established at a time. This bug is observed to be in v1.11.2221.0 and _not_ in v1.9.1942.0.
This commit is contained in:
parent
70560a789c
commit
0220f71883
|
@ -262,61 +262,73 @@ IFACEMETHODIMP UiaTextRangeBase::ExpandToEnclosingUnit(_In_ TextUnit unit) noexc
|
||||||
|
|
||||||
try
|
try
|
||||||
{
|
{
|
||||||
const auto& buffer = _pData->GetTextBuffer();
|
_expandToEnclosingUnit(unit);
|
||||||
const auto bufferSize = _getBufferSize();
|
|
||||||
const auto bufferEnd = bufferSize.EndExclusive();
|
|
||||||
|
|
||||||
if (unit == TextUnit_Character)
|
|
||||||
{
|
|
||||||
_start = buffer.GetGlyphStart(_start);
|
|
||||||
_end = buffer.GetGlyphEnd(_start);
|
|
||||||
}
|
|
||||||
else if (unit <= TextUnit_Word)
|
|
||||||
{
|
|
||||||
// expand to word
|
|
||||||
_start = buffer.GetWordStart(_start, _wordDelimiters, true);
|
|
||||||
_end = buffer.GetWordEnd(_start, _wordDelimiters, true);
|
|
||||||
|
|
||||||
// GetWordEnd may return the actual end of the TextBuffer.
|
|
||||||
// If so, just set it to this value of bufferEnd
|
|
||||||
if (!bufferSize.IsInBounds(_end))
|
|
||||||
{
|
|
||||||
_end = bufferEnd;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
else if (unit <= TextUnit_Line)
|
|
||||||
{
|
|
||||||
if (_start == bufferEnd)
|
|
||||||
{
|
|
||||||
// Special case: if we are at the bufferEnd,
|
|
||||||
// move _start back one, instead of _end forward
|
|
||||||
_start.X = 0;
|
|
||||||
_start.Y = base::ClampSub(_start.Y, 1);
|
|
||||||
_end = bufferEnd;
|
|
||||||
}
|
|
||||||
else
|
|
||||||
{
|
|
||||||
// expand to line
|
|
||||||
_start.X = 0;
|
|
||||||
_end.X = 0;
|
|
||||||
_end.Y = base::ClampAdd(_start.Y, 1);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
else
|
|
||||||
{
|
|
||||||
// TODO GH#6986: properly handle "end of buffer" as last character
|
|
||||||
// instead of last cell
|
|
||||||
// expand to document
|
|
||||||
_start = bufferSize.Origin();
|
|
||||||
_end = bufferSize.EndExclusive();
|
|
||||||
}
|
|
||||||
|
|
||||||
UiaTracing::TextRange::ExpandToEnclosingUnit(unit, *this);
|
UiaTracing::TextRange::ExpandToEnclosingUnit(unit, *this);
|
||||||
return S_OK;
|
return S_OK;
|
||||||
}
|
}
|
||||||
CATCH_RETURN();
|
CATCH_RETURN();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Method Description:
|
||||||
|
// - Moves _start and _end endpoints to encompass the enclosing text unit.
|
||||||
|
// (i.e. word --> enclosing word, line --> enclosing line)
|
||||||
|
// - IMPORTANT: this does _not_ lock the console
|
||||||
|
// Arguments:
|
||||||
|
// - attributeId - the UIA text attribute identifier we're expanding by
|
||||||
|
// Return Value:
|
||||||
|
// - <none>
|
||||||
|
void UiaTextRangeBase::_expandToEnclosingUnit(TextUnit unit)
|
||||||
|
{
|
||||||
|
const auto& buffer = _pData->GetTextBuffer();
|
||||||
|
const auto bufferSize = _getBufferSize();
|
||||||
|
const auto bufferEnd = bufferSize.EndExclusive();
|
||||||
|
|
||||||
|
if (unit == TextUnit_Character)
|
||||||
|
{
|
||||||
|
_start = buffer.GetGlyphStart(_start);
|
||||||
|
_end = buffer.GetGlyphEnd(_start);
|
||||||
|
}
|
||||||
|
else if (unit <= TextUnit_Word)
|
||||||
|
{
|
||||||
|
// expand to word
|
||||||
|
_start = buffer.GetWordStart(_start, _wordDelimiters, true);
|
||||||
|
_end = buffer.GetWordEnd(_start, _wordDelimiters, true);
|
||||||
|
|
||||||
|
// GetWordEnd may return the actual end of the TextBuffer.
|
||||||
|
// If so, just set it to this value of bufferEnd
|
||||||
|
if (!bufferSize.IsInBounds(_end))
|
||||||
|
{
|
||||||
|
_end = bufferEnd;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
else if (unit <= TextUnit_Line)
|
||||||
|
{
|
||||||
|
if (_start == bufferEnd)
|
||||||
|
{
|
||||||
|
// Special case: if we are at the bufferEnd,
|
||||||
|
// move _start back one, instead of _end forward
|
||||||
|
_start.X = 0;
|
||||||
|
_start.Y = base::ClampSub(_start.Y, 1);
|
||||||
|
_end = bufferEnd;
|
||||||
|
}
|
||||||
|
else
|
||||||
|
{
|
||||||
|
// expand to line
|
||||||
|
_start.X = 0;
|
||||||
|
_end.X = 0;
|
||||||
|
_end.Y = base::ClampAdd(_start.Y, 1);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
else
|
||||||
|
{
|
||||||
|
// TODO GH#6986: properly handle "end of buffer" as last character
|
||||||
|
// instead of last cell
|
||||||
|
// expand to document
|
||||||
|
_start = bufferSize.Origin();
|
||||||
|
_end = bufferSize.EndExclusive();
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// Method Description:
|
// Method Description:
|
||||||
// - Verify that the given attribute has the desired formatting saved in the attributeId and val
|
// - Verify that the given attribute has the desired formatting saved in the attributeId and val
|
||||||
// Arguments:
|
// Arguments:
|
||||||
|
@ -994,6 +1006,7 @@ std::wstring UiaTextRangeBase::_getTextValue(std::optional<unsigned int> maxLeng
|
||||||
IFACEMETHODIMP UiaTextRangeBase::Move(_In_ TextUnit unit,
|
IFACEMETHODIMP UiaTextRangeBase::Move(_In_ TextUnit unit,
|
||||||
_In_ int count,
|
_In_ int count,
|
||||||
_Out_ int* pRetVal) noexcept
|
_Out_ int* pRetVal) noexcept
|
||||||
|
try
|
||||||
{
|
{
|
||||||
RETURN_HR_IF(E_INVALIDARG, pRetVal == nullptr);
|
RETURN_HR_IF(E_INVALIDARG, pRetVal == nullptr);
|
||||||
*pRetVal = 0;
|
*pRetVal = 0;
|
||||||
|
@ -1011,26 +1024,22 @@ IFACEMETHODIMP UiaTextRangeBase::Move(_In_ TextUnit unit,
|
||||||
constexpr auto endpoint = TextPatternRangeEndpoint::TextPatternRangeEndpoint_Start;
|
constexpr auto endpoint = TextPatternRangeEndpoint::TextPatternRangeEndpoint_Start;
|
||||||
constexpr auto preventBufferEnd = true;
|
constexpr auto preventBufferEnd = true;
|
||||||
const auto wasDegenerate = IsDegenerate();
|
const auto wasDegenerate = IsDegenerate();
|
||||||
try
|
if (unit == TextUnit::TextUnit_Character)
|
||||||
{
|
{
|
||||||
if (unit == TextUnit::TextUnit_Character)
|
_moveEndpointByUnitCharacter(count, endpoint, pRetVal, preventBufferEnd);
|
||||||
{
|
}
|
||||||
_moveEndpointByUnitCharacter(count, endpoint, pRetVal, preventBufferEnd);
|
else if (unit <= TextUnit::TextUnit_Word)
|
||||||
}
|
{
|
||||||
else if (unit <= TextUnit::TextUnit_Word)
|
_moveEndpointByUnitWord(count, endpoint, pRetVal, preventBufferEnd);
|
||||||
{
|
}
|
||||||
_moveEndpointByUnitWord(count, endpoint, pRetVal, preventBufferEnd);
|
else if (unit <= TextUnit::TextUnit_Line)
|
||||||
}
|
{
|
||||||
else if (unit <= TextUnit::TextUnit_Line)
|
_moveEndpointByUnitLine(count, endpoint, pRetVal, preventBufferEnd);
|
||||||
{
|
}
|
||||||
_moveEndpointByUnitLine(count, endpoint, pRetVal, preventBufferEnd);
|
else if (unit <= TextUnit::TextUnit_Document)
|
||||||
}
|
{
|
||||||
else if (unit <= TextUnit::TextUnit_Document)
|
_moveEndpointByUnitDocument(count, endpoint, pRetVal, preventBufferEnd);
|
||||||
{
|
|
||||||
_moveEndpointByUnitDocument(count, endpoint, pRetVal, preventBufferEnd);
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
CATCH_RETURN();
|
|
||||||
|
|
||||||
// If we actually moved...
|
// If we actually moved...
|
||||||
if (*pRetVal != 0)
|
if (*pRetVal != 0)
|
||||||
|
@ -1044,13 +1053,14 @@ IFACEMETHODIMP UiaTextRangeBase::Move(_In_ TextUnit unit,
|
||||||
else
|
else
|
||||||
{
|
{
|
||||||
// then just expand to get our _end
|
// then just expand to get our _end
|
||||||
ExpandToEnclosingUnit(unit);
|
_expandToEnclosingUnit(unit);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
UiaTracing::TextRange::Move(unit, count, *pRetVal, *this);
|
UiaTracing::TextRange::Move(unit, count, *pRetVal, *this);
|
||||||
return S_OK;
|
return S_OK;
|
||||||
}
|
}
|
||||||
|
CATCH_RETURN();
|
||||||
|
|
||||||
IFACEMETHODIMP UiaTextRangeBase::MoveEndpointByUnit(_In_ TextPatternRangeEndpoint endpoint,
|
IFACEMETHODIMP UiaTextRangeBase::MoveEndpointByUnit(_In_ TextPatternRangeEndpoint endpoint,
|
||||||
_In_ TextUnit unit,
|
_In_ TextUnit unit,
|
||||||
|
|
|
@ -153,6 +153,8 @@ namespace Microsoft::Console::Types
|
||||||
|
|
||||||
void _getBoundingRect(const til::rectangle textRect, _Inout_ std::vector<double>& coords) const;
|
void _getBoundingRect(const til::rectangle textRect, _Inout_ std::vector<double>& coords) const;
|
||||||
|
|
||||||
|
void _expandToEnclosingUnit(TextUnit unit);
|
||||||
|
|
||||||
void
|
void
|
||||||
_moveEndpointByUnitCharacter(_In_ const int moveCount,
|
_moveEndpointByUnitCharacter(_In_ const int moveCount,
|
||||||
_In_ const TextPatternRangeEndpoint endpoint,
|
_In_ const TextPatternRangeEndpoint endpoint,
|
||||||
|
|
Loading…
Reference in a new issue