-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Fix leak in buffering text for UIA when unfocused #16251
Conversation
src/renderer/uia/UiaRenderer.cpp
Outdated
// If we had buffered any text from NotifyNewText, dump it. When we do come | ||
// back around to actually paint, we will just no-op. No sense in keeping | ||
// the data buffered. | ||
_newOutput.clear(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will not release any memory however. Additionally, it also doesn't clear _queuedOutput
. Do we need to do that?
This comment contains one approach to clear the memory in the EndPaint()
method: #16209 (comment)
In the Disable()
case I'd release the memory whenever, let's say, _newOutput.capacity() >= 4096
or something.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Derp yea, we can resize it too.
I didn't think we needed to clear out _queuedOutput
, since we might technically be after EndPaint
, but in the middle of a Present
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah right, good point.
Notes in #16217 have the investigation.
TL;DR: we'd always buffer text. Even if we're disabled (unfocused). When we're
disabled, we'd never clear the buffered text. Oops.
Closes #16217