Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Eliminate two memory leaks #16597

Merged

Conversation

glenrgordon
Copy link
Contributor

@glenrgordon glenrgordon commented Jan 24, 2024

In WindowsTerminal, there was a leak of a BSTR with every call to
ITextRangeProvider::GetText, and a failure to call VariantClear in
ITextRange::GetAttributeValue when the value stored in the variant is
VT_BSTR. These were fixed by switching to wil::unique_bstr and
wil::unique_variant.

return winrt::to_hstring(returnVal);
wil::unique_bstr returnVal;
THROW_IF_FAILED(_uiaProvider->GetText(maxLength, returnVal.put()));
return winrt::to_hstring(returnVal.get());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could choose to use return winrt::hstring{ returnVal.get(), SysStringLen(returnVal.get()) } here to avoid the need for the wcslen call.

Copy link
Member

@DHowett DHowett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Argh! Thank you for doing this.

I'll mark this as approved but wait a little bit in case you want to do the wcslen thing.

@DHowett DHowett added this to To Cherry Pick in 1.19 Servicing Pipeline via automation Jan 24, 2024
@DHowett DHowett added this to To Cherry Pick in 1.18 Servicing Pipeline via automation Jan 24, 2024
In WindowsTerminal, there was a leak of a BSTR with every call to
ITextRangeProvider::GetText, and a failure to call VariantClear in
ITextRange::GetAttributeValue when the value stored in the variant is
VT_BSTR. These were fixed by switching to wil::unique_bstr and
wil::unique_variant.
@lhecker lhecker added Issue-Bug It either shouldn't be doing this or needs an investigation. AutoMerge Marked for automatic merge by the bot when requirements are met labels Jan 25, 2024
@microsoft-github-policy-service microsoft-github-policy-service bot merged commit da99d89 into microsoft:main Jan 25, 2024
13 of 15 checks passed
@DHowett DHowett moved this from To Cherry Pick to Cherry Picked in 1.18 Servicing Pipeline Jan 25, 2024
DHowett pushed a commit that referenced this pull request Jan 25, 2024
In WindowsTerminal, there was a leak of a BSTR with every call to
ITextRangeProvider::GetText, and a failure to call VariantClear in
ITextRange::GetAttributeValue when the value stored in the variant is
VT_BSTR. These were fixed by switching to wil::unique_bstr and
wil::unique_variant.

(cherry picked from commit da99d89)
Service-Card-Id: 91631735
Service-Version: 1.18
@DHowett DHowett moved this from To Cherry Pick to Cherry Picked in 1.19 Servicing Pipeline Jan 25, 2024
DHowett pushed a commit that referenced this pull request Jan 25, 2024
In WindowsTerminal, there was a leak of a BSTR with every call to
ITextRangeProvider::GetText, and a failure to call VariantClear in
ITextRange::GetAttributeValue when the value stored in the variant is
VT_BSTR. These were fixed by switching to wil::unique_bstr and
wil::unique_variant.

(cherry picked from commit da99d89)
Service-Card-Id: 91631736
Service-Version: 1.19
@DHowett DHowett moved this from Cherry Picked to Validated in 1.19 Servicing Pipeline Feb 21, 2024
@DHowett DHowett moved this from Validated to Shipped in 1.19 Servicing Pipeline Feb 21, 2024
@DHowett DHowett moved this from Cherry Picked to Shipped in 1.18 Servicing Pipeline Feb 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AutoMerge Marked for automatic merge by the bot when requirements are met Issue-Bug It either shouldn't be doing this or needs an investigation.
Development

Successfully merging this pull request may close these issues.

None yet

3 participants