Skip to content

Commit 3849321

Browse files
authored
Fix a crash in _makeCursorVisible (#19329)
Fixes the crash and also makes `SnapOnOutput` a bit nicer. Closes #19325 ## Validation Steps Performed * Launch vim in WSL * Exit * No crash ✅
1 parent 5a05f82 commit 3849321

File tree

2 files changed

+33
-10
lines changed

2 files changed

+33
-10
lines changed

src/host/screenInfo.cpp

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1718,6 +1718,19 @@ void SCREEN_INFORMATION::SnapOnInput(const WORD vkey)
17181718
}
17191719
}
17201720

1721+
SCREEN_INFORMATION::SnapOnScopeExit SCREEN_INFORMATION::SnapOnOutput() noexcept
1722+
{
1723+
const auto call =
1724+
// We don't need to snap-on-output the alt buffer since it doesn't scroll anyway.
1725+
// More importantly though, in the current architecture the alt buffer gets deallocated
1726+
// the second we receive a \033[?1049l, which makes our this pointer hazardous to use.
1727+
_psiMainBuffer == nullptr &&
1728+
// We only want to snap if the user didn't intentionally scroll away.
1729+
// The snapping logic could be improved, but it's alright for now.
1730+
_viewport.IsInBounds(_textBuffer->GetCursor().GetPosition());
1731+
return SnapOnScopeExit{ call ? this : nullptr };
1732+
}
1733+
17211734
void SCREEN_INFORMATION::_makeCursorVisible()
17221735
{
17231736
if (_textBuffer->GetCursor().IsVisible())

src/host/screenInfo.hpp

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,25 @@ class ConversionAreaInfo; // forward decl window. circular reference
5050
class SCREEN_INFORMATION : public ConsoleObjectHeader, public Microsoft::Console::IIoProvider
5151
{
5252
public:
53+
// This little helper works like wil::scope_exit but is slimmer
54+
// (= easier to optimize) and has a concrete type (= can declare).
55+
struct SnapOnScopeExit
56+
{
57+
~SnapOnScopeExit()
58+
{
59+
if (self)
60+
{
61+
try
62+
{
63+
self->_makeCursorVisible();
64+
}
65+
CATCH_LOG();
66+
}
67+
}
68+
69+
SCREEN_INFORMATION* self;
70+
};
71+
5372
[[nodiscard]] static NTSTATUS CreateInstance(_In_ til::size coordWindowSize,
5473
const FontInfo fontInfo,
5574
_In_ til::size coordScreenBufferSize,
@@ -73,16 +92,7 @@ class SCREEN_INFORMATION : public ConsoleObjectHeader, public Microsoft::Console
7392
void MakeCurrentCursorVisible();
7493
void MakeCursorVisible(til::point position);
7594
void SnapOnInput(WORD vkey);
76-
auto SnapOnOutput() noexcept
77-
{
78-
const auto inBounds = _viewport.IsInBounds(_textBuffer->GetCursor().GetPosition());
79-
return wil::scope_exit([this, inBounds]() {
80-
if (inBounds)
81-
{
82-
_makeCursorVisible();
83-
}
84-
});
85-
}
95+
SnapOnScopeExit SnapOnOutput() noexcept;
8696

8797
void ClipToScreenBuffer(_Inout_ til::inclusive_rect* const psrClip) const;
8898

0 commit comments

Comments
 (0)