From 4370da9549dc2e51ce7e03f590d8ad9fcf2fae77 Mon Sep 17 00:00:00 2001 From: Leonard Hecker Date: Tue, 19 Sep 2023 18:59:39 +0200 Subject: [PATCH] Add missing lock guards on Terminal access (#15894) `Terminal` is used concurrently by at least 4 threads. The table below lists the class members and the threads that access them to the best of my knowledge. Where: * UI: UI Thread * BG: Background worker threads (`winrt::resume_background`) * RD: Render thread * VT: VT connection thread | | UI | BG | RD | VT | |------------------------------------|----|----|----|----| | `_pfnWriteInput` | x | x | | x | | `_pfnWarningBell` | | | | x | | `_pfnTitleChanged` | | | | x | | `_pfnCopyToClipboard` | | | | x | | `_pfnScrollPositionChanged` | x | x | | x | | `_pfnCursorPositionChanged` | | | | x | | `_pfnTaskbarProgressChanged` | | | | x | | `_pfnShowWindowChanged` | | | | x | | `_pfnPlayMidiNote` | | | | x | | `_pfnCompletionsChanged` | | | | x | | `_renderSettings` | x | | x | x | | `_stateMachine` | x | | | x | | `_terminalInput` | x | | | x | | `_title` | x | | x | x | | `_startingTitle` | x | | x | | | `_startingTabColor` | x | | | | | `_defaultCursorShape` | x | | | x | | `_systemMode` | | x | x | x | | `_snapOnInput` | x | x | | | | `_altGrAliasing` | x | | | | | `_suppressApplicationTitle` | x | | | x | | `_trimBlockSelection` | x | | | | | `_autoMarkPrompts` | x | | | | | `_taskbarState` | x | | | x | | `_taskbarProgress` | x | | | x | | `_workingDirectory` | x | | | x | | `_fontInfo` | x | | x | | | `_selection` | x | x | x | x | | `_blockSelection` | x | x | x | | | `_wordDelimiters` | x | x | | | | `_multiClickSelectionMode` | x | x | x | | | `_selectionMode` | x | x | x | | | `_selectionIsTargetingUrl` | x | x | x | | | `_selectionEndpoint` | x | x | x | | | `_anchorInactiveSelectionEndpoint` | x | x | x | | | `_mainBuffer` | x | x | x | x | | `_altBuffer` | x | x | x | x | | `_mutableViewport` | x | | x | x | | `_scrollbackLines` | x | | | | | `_detectURLs` | x | | | | | `_altBufferSize` | x | x | x | x | | `_deferredResize` | x | | | x | | `_scrollOffset` | x | x | x | x | | `_patternIntervalTree` | x | x | x | x | | `_lastKeyEventCodes` | x | | | | | `_currentPromptState` | x | | | x | Only 7 members are specific to one thread and don't require locking. All other members require some for of locking to be safe for use. To address the issue this changeset adds `LockForReading/LockForWriting` calls everywhere `_terminal` is accessed in `ControlCore/HwndTerminal`. Additionally, to ensure these issues don't pop up anymore, it adds to all `Terminal` functions a debug assertion that the lock is being held. Finally, because this changeset started off rather modest, it contains changes that I initially made without being aware about the extent of the issue. It simplifies the access around `_patternIntervalTree` by making `_InvalidatePatternTree()` directly use that member. Furthermore, it simplifies `_terminal->SetCursorOn(!IsCursorOn())` to `BlinkCursor()`, allowing the code to be shared with `HwndTerminal`. Ideally `Terminal` should not be that much of a class so that we don't need such coarse locking. Splitting out selection and rendering state should allow deduplicating code with conhost and use finer locking. Closes #9617 ## Validation Steps Performed I tried to use as many Windows Terminal features as I could and fixed every occurrence of `_assertLocked()` failures. --- .github/actions/spelling/allow/apis.txt | 1 + .../PublicTerminalCore/HwndTerminal.cpp | 176 +++++++----- .../PublicTerminalCore/HwndTerminal.hpp | 3 +- src/cascadia/TerminalControl/ControlCore.cpp | 186 ++++++++----- src/cascadia/TerminalCore/Terminal.cpp | 257 +++++++++++------- src/cascadia/TerminalCore/Terminal.hpp | 24 +- src/cascadia/TerminalCore/TerminalApi.cpp | 24 +- .../TerminalCore/TerminalSelection.cpp | 11 +- .../TerminalCore/terminalrenderdata.cpp | 16 +- 9 files changed, 436 insertions(+), 262 deletions(-) diff --git a/.github/actions/spelling/allow/apis.txt b/.github/actions/spelling/allow/apis.txt index 71b2922ca03..e58df6e202b 100644 --- a/.github/actions/spelling/allow/apis.txt +++ b/.github/actions/spelling/allow/apis.txt @@ -28,6 +28,7 @@ CYICON Dacl dataobject dcomp +debugbreak delayimp DERR dlldata diff --git a/src/cascadia/PublicTerminalCore/HwndTerminal.cpp b/src/cascadia/PublicTerminalCore/HwndTerminal.cpp index ad4e753c9a6..a13ac5b3eda 100644 --- a/src/cascadia/PublicTerminalCore/HwndTerminal.cpp +++ b/src/cascadia/PublicTerminalCore/HwndTerminal.cpp @@ -43,13 +43,13 @@ try return DefWindowProc(hwnd, WM_NCCREATE, wParam, lParam); } #pragma warning(suppress : 26490) // Win32 APIs can only store void*, have to use reinterpret_cast - auto terminal = reinterpret_cast(GetWindowLongPtr(hwnd, GWLP_USERDATA)); + const auto publicTerminal = reinterpret_cast(GetWindowLongPtr(hwnd, GWLP_USERDATA)); - if (terminal) + if (publicTerminal) { if (_IsMouseMessage(uMsg)) { - if (terminal->_CanSendVTMouseInput() && terminal->_SendMouseEvent(uMsg, wParam, lParam)) + if (publicTerminal->_CanSendVTMouseInput() && publicTerminal->_SendMouseEvent(uMsg, wParam, lParam)) { // GH#6401: Capturing the mouse ensures that we get drag/release events // even if the user moves outside the window. @@ -81,14 +81,14 @@ try case WM_GETOBJECT: if (lParam == UiaRootObjectId) { - return UiaReturnRawElementProvider(hwnd, wParam, lParam, terminal->_GetUiaProvider()); + return UiaReturnRawElementProvider(hwnd, wParam, lParam, publicTerminal->_GetUiaProvider()); } break; case WM_LBUTTONDOWN: - LOG_IF_FAILED(terminal->_StartSelection(lParam)); + LOG_IF_FAILED(publicTerminal->_StartSelection(lParam)); return 0; case WM_LBUTTONUP: - terminal->_singleClickTouchdownPos = std::nullopt; + publicTerminal->_singleClickTouchdownPos = std::nullopt; [[fallthrough]]; case WM_MBUTTONUP: case WM_RBUTTONUP: @@ -97,30 +97,31 @@ try case WM_MOUSEMOVE: if (WI_IsFlagSet(wParam, MK_LBUTTON)) { - LOG_IF_FAILED(terminal->_MoveSelection(lParam)); + LOG_IF_FAILED(publicTerminal->_MoveSelection(lParam)); return 0; } break; case WM_RBUTTONDOWN: - if (const auto& termCore{ terminal->_terminal }; termCore && termCore->IsSelectionActive()) + if (publicTerminal->_terminal && publicTerminal->_terminal->IsSelectionActive()) { try { - const auto bufferData = termCore->RetrieveSelectedTextFromBuffer(false); - LOG_IF_FAILED(terminal->_CopyTextToSystemClipboard(bufferData, true)); - TerminalClearSelection(terminal); + const auto lock = publicTerminal->_terminal->LockForWriting(); + const auto bufferData = publicTerminal->_terminal->RetrieveSelectedTextFromBuffer(false); + LOG_IF_FAILED(publicTerminal->_CopyTextToSystemClipboard(bufferData, true)); + publicTerminal->_ClearSelection(); } CATCH_LOG(); } else { - terminal->_PasteTextFromClipboard(); + publicTerminal->_PasteTextFromClipboard(); } return 0; case WM_DESTROY: // Release Terminal's hwnd so Teardown doesn't try to destroy it again - terminal->_hwnd.release(); - terminal->Teardown(); + publicTerminal->_hwnd.release(); + publicTerminal->Teardown(); return 0; default: break; @@ -195,6 +196,8 @@ HwndTerminal::~HwndTerminal() HRESULT HwndTerminal::Initialize() { _terminal = std::make_unique<::Microsoft::Terminal::Core::Terminal>(); + const auto lock = _terminal->LockForWriting(); + auto renderThread = std::make_unique<::Microsoft::Console::Render::RenderThread>(); auto* const localPointerToThread = renderThread.get(); auto& renderSettings = _terminal->GetRenderSettings(); @@ -304,7 +307,6 @@ void HwndTerminal::_UpdateFont(int newDpi) return; } _currentDpi = newDpi; - auto lock = _terminal->LockForWriting(); // TODO: MSFT:20895307 If the font doesn't exist, this doesn't // actually fail. We need a way to gracefully fallback. @@ -323,10 +325,10 @@ IRawElementProviderSimple* HwndTerminal::_GetUiaProvider() noexcept { return nullptr; } - auto lock = _terminal->LockForWriting(); LOG_IF_FAILED(::Microsoft::WRL::MakeAndInitialize(&_uiaProvider, this->GetRenderData(), this)); _uiaEngine = std::make_unique<::Microsoft::Console::Render::UiaEngine>(_uiaProvider.Get()); LOG_IF_FAILED(_uiaEngine->Enable()); + const auto lock = _terminal->LockForWriting(); _renderer->AddRenderEngine(_uiaEngine.get()); } catch (...) @@ -344,7 +346,7 @@ HRESULT HwndTerminal::Refresh(const til::size windowSize, _Out_ til::size* dimen RETURN_HR_IF_NULL(E_NOT_VALID_STATE, _terminal); RETURN_HR_IF_NULL(E_INVALIDARG, dimensions); - auto lock = _terminal->LockForWriting(); + const auto lock = _terminal->LockForWriting(); _terminal->ClearSelection(); @@ -381,37 +383,45 @@ void HwndTerminal::SendOutput(std::wstring_view data) { return; } + const auto lock = _terminal->LockForWriting(); _terminal->Write(data); } HRESULT _stdcall CreateTerminal(HWND parentHwnd, _Out_ void** hwnd, _Out_ void** terminal) { - auto _terminal = std::make_unique(parentHwnd); - RETURN_IF_FAILED(_terminal->Initialize()); + auto publicTerminal = std::make_unique(parentHwnd); + + RETURN_IF_FAILED(publicTerminal->Initialize()); - *hwnd = _terminal->GetHwnd(); - *terminal = _terminal.release(); + *hwnd = publicTerminal->GetHwnd(); + *terminal = publicTerminal.release(); return S_OK; } void _stdcall TerminalRegisterScrollCallback(void* terminal, void __stdcall callback(int, int, int)) +try { - auto publicTerminal = static_cast(terminal); + const auto publicTerminal = static_cast(terminal); publicTerminal->RegisterScrollCallback(callback); } +CATCH_LOG() void _stdcall TerminalRegisterWriteCallback(void* terminal, const void __stdcall callback(wchar_t*)) +try { const auto publicTerminal = static_cast(terminal); publicTerminal->RegisterWriteCallback(callback); } +CATCH_LOG() void _stdcall TerminalSendOutput(void* terminal, LPCWSTR data) +try { const auto publicTerminal = static_cast(terminal); publicTerminal->SendOutput(data); } +CATCH_LOG() /// /// Triggers a terminal resize using the new width and height in pixel. @@ -446,13 +456,18 @@ HRESULT _stdcall TerminalTriggerResize(_In_ void* terminal, _In_ til::CoordType /// Out parameter with the new size of the renderer. /// HRESULT of the attempted resize. HRESULT _stdcall TerminalTriggerResizeWithDimension(_In_ void* terminal, _In_ til::size dimensionsInCharacters, _Out_ til::size* dimensionsInPixels) +try { RETURN_HR_IF_NULL(E_INVALIDARG, dimensionsInPixels); const auto publicTerminal = static_cast(terminal); - const auto viewInCharacters = Viewport::FromDimensions(dimensionsInCharacters); - const auto viewInPixels = publicTerminal->_renderEngine->GetViewportInPixels(viewInCharacters); + Viewport viewInPixels; + { + const auto viewInCharacters = Viewport::FromDimensions(dimensionsInCharacters); + const auto lock = publicTerminal->_terminal->LockForReading(); + viewInPixels = publicTerminal->_renderEngine->GetViewportInPixels(viewInCharacters); + } dimensionsInPixels->width = viewInPixels.Width(); dimensionsInPixels->height = viewInPixels.Height(); @@ -461,6 +476,7 @@ HRESULT _stdcall TerminalTriggerResizeWithDimension(_In_ void* terminal, _In_ ti return TerminalTriggerResize(terminal, viewInPixels.Width(), viewInPixels.Height(), &unused); } +CATCH_RETURN() /// /// Calculates the amount of rows and columns that fit in the provided width and height. @@ -471,10 +487,12 @@ HRESULT _stdcall TerminalTriggerResizeWithDimension(_In_ void* terminal, _In_ ti /// Out parameter containing the columns and rows that fit the new size. /// HRESULT of the calculation. HRESULT _stdcall TerminalCalculateResize(_In_ void* terminal, _In_ til::CoordType width, _In_ til::CoordType height, _Out_ til::size* dimensions) +try { const auto publicTerminal = static_cast(terminal); const auto viewInPixels = Viewport::FromDimensions({ width, height }); + const auto lock = publicTerminal->_terminal->LockForReading(); const auto viewInCharacters = publicTerminal->_renderEngine->GetViewportInCharacters(viewInPixels); dimensions->width = viewInCharacters.Width(); @@ -482,20 +500,27 @@ HRESULT _stdcall TerminalCalculateResize(_In_ void* terminal, _In_ til::CoordTyp return S_OK; } +CATCH_RETURN() void _stdcall TerminalDpiChanged(void* terminal, int newDpi) +try { const auto publicTerminal = static_cast(terminal); + const auto lock = publicTerminal->_terminal->LockForWriting(); publicTerminal->_UpdateFont(newDpi); } +CATCH_LOG() void _stdcall TerminalUserScroll(void* terminal, int viewTop) +try { if (const auto publicTerminal = static_cast(terminal); publicTerminal && publicTerminal->_terminal) { + const auto lock = publicTerminal->_terminal->LockForWriting(); publicTerminal->_terminal->UserScrollViewport(viewTop); } } +CATCH_LOG() const unsigned int HwndTerminal::_NumberOfClicks(til::point point, std::chrono::steady_clock::time_point timestamp) noexcept { @@ -522,7 +547,7 @@ try GET_Y_LPARAM(lParam), }; - auto lock = _terminal->LockForWriting(); + const auto lock = _terminal->LockForWriting(); const auto altPressed = GetKeyState(VK_MENU) < 0; const til::size fontSize{ this->_actualFont.GetSize() }; @@ -566,7 +591,7 @@ try GET_Y_LPARAM(lParam), }; - auto lock = _terminal->LockForWriting(); + const auto lock = _terminal->LockForWriting(); const til::size fontSize{ this->_actualFont.GetSize() }; RETURN_HR_IF(E_NOT_VALID_STATE, fontSize.area() == 0); // either dimension = 0, area == 0 @@ -596,45 +621,57 @@ try } CATCH_RETURN(); -void HwndTerminal::_ClearSelection() noexcept -try +void HwndTerminal::_ClearSelection() { if (!_terminal) { return; } - auto lock{ _terminal->LockForWriting() }; _terminal->ClearSelection(); _renderer->TriggerSelection(); } -CATCH_LOG(); void _stdcall TerminalClearSelection(void* terminal) +try { - auto publicTerminal = static_cast(terminal); + const auto publicTerminal = static_cast(terminal); + const auto lock = publicTerminal->_terminal->LockForWriting(); publicTerminal->_ClearSelection(); } +CATCH_LOG() bool _stdcall TerminalIsSelectionActive(void* terminal) +try { if (const auto publicTerminal = static_cast(terminal); publicTerminal && publicTerminal->_terminal) { + const auto lock = publicTerminal->_terminal->LockForReading(); return publicTerminal->_terminal->IsSelectionActive(); } return false; } +catch (...) +{ + LOG_CAUGHT_EXCEPTION(); + return false; +} // Returns the selected text in the terminal. const wchar_t* _stdcall TerminalGetSelection(void* terminal) +try { - auto publicTerminal = static_cast(terminal); + const auto publicTerminal = static_cast(terminal); if (!publicTerminal || !publicTerminal->_terminal) { return nullptr; } - const auto bufferData = publicTerminal->_terminal->RetrieveSelectedTextFromBuffer(false); - publicTerminal->_ClearSelection(); + TextBuffer::TextAndColor bufferData; + { + const auto lock = publicTerminal->_terminal->LockForWriting(); + bufferData = publicTerminal->_terminal->RetrieveSelectedTextFromBuffer(false); + publicTerminal->_ClearSelection(); + } // convert text: vector --> string std::wstring selectedText; @@ -646,6 +683,11 @@ const wchar_t* _stdcall TerminalGetSelection(void* terminal) auto returnText = wil::make_cotaskmem_string_nothrow(selectedText.c_str()); return returnText.release(); } +catch (...) +{ + LOG_CAUGHT_EXCEPTION(); + return nullptr; +} static ControlKeyStates getControlKeyState() noexcept { @@ -683,6 +725,7 @@ bool HwndTerminal::_CanSendVTMouseInput() const noexcept { // Only allow the transit of mouse events if shift isn't pressed. const auto shiftPressed = GetKeyState(VK_SHIFT) < 0; + const auto lock = _terminal->LockForReading(); return !shiftPressed && _focused && _terminal && _terminal->IsTrackingMouseInput(); } @@ -715,6 +758,7 @@ try WI_IsFlagSet(GetKeyState(VK_RBUTTON), KeyPressed) }; + const auto lock = _terminal->LockForWriting(); return _terminal->SendMouseEvent(cursorPosition / fontSize, uMsg, getControlKeyState(), wheelDelta, state); } catch (...) @@ -740,6 +784,7 @@ try { _uiaProvider->RecordKeyEvent(vkey); } + const auto lock = _terminal->LockForWriting(); _terminal->SendKeyEvent(vkey, scanCode, modifiers, keyDown); } CATCH_LOG(); @@ -751,7 +796,10 @@ try { return; } - else if (_terminal->IsSelectionActive()) + + const auto lock = _terminal->LockForWriting(); + + if (_terminal->IsSelectionActive()) { _ClearSelection(); if (ch == UNICODE_ESC) @@ -778,16 +826,20 @@ try CATCH_LOG(); void _stdcall TerminalSendKeyEvent(void* terminal, WORD vkey, WORD scanCode, WORD flags, bool keyDown) +try { const auto publicTerminal = static_cast(terminal); publicTerminal->_SendKeyEvent(vkey, scanCode, flags, keyDown); } +CATCH_LOG() void _stdcall TerminalSendCharEvent(void* terminal, wchar_t ch, WORD scanCode, WORD flags) +try { const auto publicTerminal = static_cast(terminal); publicTerminal->_SendCharEvent(ch, scanCode, flags); } +CATCH_LOG() void _stdcall DestroyTerminal(void* terminal) { @@ -803,8 +855,9 @@ void _stdcall TerminalSetTheme(void* terminal, TerminalTheme theme, LPCWSTR font { return; } + { - auto lock = publicTerminal->_terminal->LockForWriting(); + const auto lock = publicTerminal->_terminal->LockForWriting(); auto& renderSettings = publicTerminal->_terminal->GetRenderSettings(); renderSettings.SetColorTableEntry(TextColor::DEFAULT_FOREGROUND, theme.DefaultForeground); @@ -818,12 +871,12 @@ void _stdcall TerminalSetTheme(void* terminal, TerminalTheme theme, LPCWSTR font // It's using gsl::at to check the index is in bounds, but the analyzer still calls this array-to-pointer-decay [[gsl::suppress(bounds .3)]] renderSettings.SetColorTableEntry(tableIndex, gsl::at(theme.ColorTable, tableIndex)); } - } - publicTerminal->_terminal->SetCursorStyle(static_cast(theme.CursorStyle)); + publicTerminal->_terminal->SetCursorStyle(static_cast(theme.CursorStyle)); - publicTerminal->_desiredFont = { fontFamily, 0, DEFAULT_FONT_WEIGHT, static_cast(fontSize), CP_UTF8 }; - publicTerminal->_UpdateFont(newDpi); + publicTerminal->_desiredFont = { fontFamily, 0, DEFAULT_FONT_WEIGHT, static_cast(fontSize), CP_UTF8 }; + publicTerminal->_UpdateFont(newDpi); + } // When the font changes the terminal dimensions need to be recalculated since the available row and column // space will have changed. @@ -836,29 +889,35 @@ void _stdcall TerminalSetTheme(void* terminal, TerminalTheme theme, LPCWSTR font } void _stdcall TerminalBlinkCursor(void* terminal) +try { const auto publicTerminal = static_cast(terminal); - if (!publicTerminal || !publicTerminal->_terminal || (!publicTerminal->_terminal->IsCursorBlinkingAllowed() && publicTerminal->_terminal->IsCursorVisible())) + if (!publicTerminal || !publicTerminal->_terminal) { return; } - publicTerminal->_terminal->SetCursorOn(!publicTerminal->_terminal->IsCursorOn()); + const auto lock = publicTerminal->_terminal->LockForWriting(); + publicTerminal->_terminal->BlinkCursor(); } +CATCH_LOG() void _stdcall TerminalSetCursorVisible(void* terminal, const bool visible) +try { const auto publicTerminal = static_cast(terminal); if (!publicTerminal || !publicTerminal->_terminal) { return; } + const auto lock = publicTerminal->_terminal->LockForWriting(); publicTerminal->_terminal->SetCursorOn(visible); } +CATCH_LOG() void __stdcall TerminalSetFocus(void* terminal) { - auto publicTerminal = static_cast(terminal); + const auto publicTerminal = static_cast(terminal); publicTerminal->_focused = true; if (auto uiaEngine = publicTerminal->_uiaEngine.get()) { @@ -868,7 +927,7 @@ void __stdcall TerminalSetFocus(void* terminal) void __stdcall TerminalKillFocus(void* terminal) { - auto publicTerminal = static_cast(terminal); + const auto publicTerminal = static_cast(terminal); publicTerminal->_focused = false; if (auto uiaEngine = publicTerminal->_uiaEngine.get()) { @@ -923,7 +982,11 @@ try { const auto& fontData = _actualFont; const int iFontHeightPoints = fontData.GetUnscaledSize().height; // this renderer uses points already - const auto bgColor = _terminal->GetAttributeColors({}).second; + COLORREF bgColor; + { + const auto lock = _terminal->LockForReading(); + bgColor = _terminal->GetAttributeColors({}).second; + } auto HTMLToPlaceOnClip = TextBuffer::GenHTML(rows, iFontHeightPoints, fontData.GetFaceName(), bgColor); _CopyToSystemClipboard(HTMLToPlaceOnClip, L"HTML Format"); @@ -993,30 +1056,16 @@ void HwndTerminal::_PasteTextFromClipboard() noexcept return; } - auto pwstr = static_cast(GlobalLock(ClipboardDataHandle)); - - _StringPaste(pwstr); + if (const auto pwstr = static_cast(GlobalLock(ClipboardDataHandle))) + { + _WriteTextToConnection(pwstr); + } GlobalUnlock(ClipboardDataHandle); CloseClipboard(); } -void HwndTerminal::_StringPaste(const wchar_t* const pData) noexcept -{ - if (pData == nullptr) - { - return; - } - - try - { - std::wstring text(pData); - _WriteTextToConnection(text); - } - CATCH_LOG(); -} - til::size HwndTerminal::GetFontSize() const noexcept { return _actualFont.GetSize(); @@ -1045,6 +1094,7 @@ void HwndTerminal::ChangeViewport(const til::inclusive_rect& NewWindow) { return; } + const auto lock = _terminal->LockForWriting(); _terminal->UserScrollViewport(NewWindow.top); } diff --git a/src/cascadia/PublicTerminalCore/HwndTerminal.hpp b/src/cascadia/PublicTerminalCore/HwndTerminal.hpp index ab3da289587..b88acd03504 100644 --- a/src/cascadia/PublicTerminalCore/HwndTerminal.hpp +++ b/src/cascadia/PublicTerminalCore/HwndTerminal.hpp @@ -112,14 +112,13 @@ struct HwndTerminal : ::Microsoft::Console::Types::IControlAccessibilityInfo HRESULT _CopyTextToSystemClipboard(const TextBuffer::TextAndColor& rows, const bool fAlsoCopyFormatting); HRESULT _CopyToSystemClipboard(std::string stringToCopy, LPCWSTR lpszFormat); void _PasteTextFromClipboard() noexcept; - void _StringPaste(const wchar_t* const pData) noexcept; const unsigned int _NumberOfClicks(til::point clickPos, std::chrono::steady_clock::time_point clickTime) noexcept; HRESULT _StartSelection(LPARAM lParam) noexcept; HRESULT _MoveSelection(LPARAM lParam) noexcept; IRawElementProviderSimple* _GetUiaProvider() noexcept; - void _ClearSelection() noexcept; + void _ClearSelection(); bool _CanSendVTMouseInput() const noexcept; bool _SendMouseEvent(UINT uMsg, WPARAM wParam, LPARAM lParam) noexcept; diff --git a/src/cascadia/TerminalControl/ControlCore.cpp b/src/cascadia/TerminalControl/ControlCore.cpp index 52dc7d4087f..31462753ff0 100644 --- a/src/cascadia/TerminalControl/ControlCore.cpp +++ b/src/cascadia/TerminalControl/ControlCore.cpp @@ -86,6 +86,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation { _settings = winrt::make_self(settings, unfocusedAppearance); _terminal = std::make_shared<::Microsoft::Terminal::Core::Terminal>(); + const auto lock = _terminal->LockForWriting(); _setupDispatcherAndCallbacks(); @@ -197,7 +198,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation [weakTerminal = std::weak_ptr{ _terminal }]() { if (const auto t = weakTerminal.lock()) { - auto lock = t->LockForWriting(); + const auto lock = t->LockForWriting(); t->UpdatePatternsUnderLock(); } }); @@ -282,6 +283,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation // Get our current size in rows/cols, and hook them up to // this connection too. { + const auto lock = _terminal->LockForReading(); const auto vp = _terminal->GetViewport(); const auto width = vp.Width(); const auto height = vp.Height(); @@ -318,7 +320,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation _compositionScale = compositionScale; { // scope for terminalLock - auto terminalLock = _terminal->LockForWriting(); + const auto lock = _terminal->LockForWriting(); if (_initializedTerminal.load(std::memory_order_relaxed)) { @@ -427,6 +429,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation { if (_initializedTerminal.load(std::memory_order_relaxed)) { + const auto lock = _terminal->LockForWriting(); _renderer->EnablePainting(); } } @@ -489,6 +492,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation _handleControlC(); } + const auto lock = _terminal->LockForWriting(); return _terminal->SendCharEvent(ch, scanCode, modifiers); } @@ -517,18 +521,19 @@ namespace winrt::Microsoft::Terminal::Control::implementation // modifier key. We'll wait for a real keystroke to dismiss the // GH #7395 - don't update selection when taking PrintScreen // selection. - return HasSelection() && ::Microsoft::Terminal::Core::Terminal::IsInputKey(vkey); + return _terminal->IsSelectionActive() && ::Microsoft::Terminal::Core::Terminal::IsInputKey(vkey); } bool ControlCore::TryMarkModeKeybinding(const WORD vkey, const ::Microsoft::Terminal::Core::ControlKeyStates mods) { + auto lock = _terminal->LockForWriting(); + if (_shouldTryUpdateSelection(vkey) && _terminal->SelectionMode() == ::Terminal::SelectionInteractionMode::Mark) { if (vkey == 'A' && !mods.IsAltPressed() && !mods.IsShiftPressed() && mods.IsCtrlPressed()) { // Ctrl + A --> Select all - auto lock = _terminal->LockForWriting(); _terminal->SelectAll(); _updateSelectionUI(); return true; @@ -536,7 +541,6 @@ namespace winrt::Microsoft::Terminal::Control::implementation else if (vkey == VK_TAB && !mods.IsAltPressed() && !mods.IsCtrlPressed() && _settings->DetectURLs()) { // [Shift +] Tab --> next/previous hyperlink - auto lock = _terminal->LockForWriting(); const auto direction = mods.IsShiftPressed() ? ::Terminal::SearchDirection::Backward : ::Terminal::SearchDirection::Forward; _terminal->SelectHyperlink(direction); _updateSelectionUI(); @@ -545,14 +549,15 @@ namespace winrt::Microsoft::Terminal::Control::implementation else if (vkey == VK_RETURN && mods.IsCtrlPressed() && !mods.IsAltPressed() && !mods.IsShiftPressed()) { // Ctrl + Enter --> Open URL - auto lock = _terminal->LockForReading(); if (const auto uri = _terminal->GetHyperlinkAtBufferPosition(_terminal->GetSelectionAnchor()); !uri.empty()) { + lock.unlock(); _OpenHyperlinkHandlers(*this, winrt::make(winrt::hstring{ uri })); } else { const auto selectedText = _terminal->GetTextBuffer().GetPlainText(_terminal->GetSelectionAnchor(), _terminal->GetSelectionEnd()); + lock.unlock(); _OpenHyperlinkHandlers(*this, winrt::make(winrt::hstring{ selectedText })); } return true; @@ -560,7 +565,6 @@ namespace winrt::Microsoft::Terminal::Control::implementation else if (vkey == VK_RETURN && !mods.IsCtrlPressed() && !mods.IsAltPressed()) { // [Shift +] Enter --> copy text - // Don't lock here! CopySelectionToClipboard already locks for you! CopySelectionToClipboard(mods.IsShiftPressed(), nullptr); _terminal->ClearSelection(); _updateSelectionUI(); @@ -575,7 +579,6 @@ namespace winrt::Microsoft::Terminal::Control::implementation else if (const auto updateSlnParams{ _terminal->ConvertKeyEventToUpdateSelectionParams(mods, vkey) }) { // try to update the selection - auto lock = _terminal->LockForWriting(); _terminal->UpdateSelection(updateSlnParams->first, updateSlnParams->second, mods); _updateSelectionUI(); return true; @@ -599,6 +602,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation const ControlKeyStates modifiers, const bool keyDown) { + const auto lock = _terminal->LockForWriting(); + // Update the selection, if it's present // GH#8522, GH#3758 - Only modify the selection on key _down_. If we // modify on key up, then there's chance that we'll immediately dismiss @@ -608,7 +613,6 @@ namespace winrt::Microsoft::Terminal::Control::implementation // try to update the selection if (const auto updateSlnParams{ _terminal->ConvertKeyEventToUpdateSelectionParams(modifiers, vkey) }) { - auto lock = _terminal->LockForWriting(); _terminal->UpdateSelection(updateSlnParams->first, updateSlnParams->second, modifiers); _updateSelectionUI(); return true; @@ -646,17 +650,18 @@ namespace winrt::Microsoft::Terminal::Control::implementation const short wheelDelta, const TerminalInput::MouseButtonState state) { + const auto lock = _terminal->LockForWriting(); return _terminal->SendMouseEvent(viewportPos, uiButton, states, wheelDelta, state); } void ControlCore::UserScrollViewport(const int viewTop) { - // Clear the regex pattern tree so the renderer does not try to render them while scrolling - _terminal->ClearPatternTree(); - - // This is a scroll event that wasn't initiated by the terminal - // itself - it was initiated by the mouse wheel, or the scrollbar. - _terminal->UserScrollViewport(viewTop); + { + // This is a scroll event that wasn't initiated by the terminal + // itself - it was initiated by the mouse wheel, or the scrollbar. + const auto lock = _terminal->LockForWriting(); + _terminal->UserScrollViewport(viewTop); + } const auto shared = _shared.lock_shared(); if (shared->updatePatternLocations) @@ -696,6 +701,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation // cleartype -> grayscale if the BG is transparent / acrylic. if (_renderEngine) { + const auto lock = _terminal->LockForWriting(); _renderEngine->EnableTransparentBackground(_isBackgroundTransparent()); _renderer->NotifyPaintFrame(); } @@ -707,7 +713,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation void ControlCore::ToggleShaderEffects() { const auto path = _settings->PixelShaderPath(); - auto lock = _terminal->LockForWriting(); + const auto lock = _terminal->LockForWriting(); // Originally, this action could be used to enable the retro effects // even when they're set to `false` in the settings. If the user didn't // specify a custom pixel shader, manually enable the legacy retro @@ -755,7 +761,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation decltype(_terminal->GetHyperlinkIntervalFromViewportPosition({})) newInterval{ std::nullopt }; if (terminalPosition.has_value()) { - auto lock = _terminal->LockForReading(); // Lock for the duration of our reads. + const auto lock = _terminal->LockForReading(); newId = _terminal->GetHyperlinkIdAtViewportPosition(*terminalPosition); newInterval = _terminal->GetHyperlinkIntervalFromViewportPosition(*terminalPosition); } @@ -770,7 +776,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation // wouldn't be able to ask us about the hyperlink text/position // without deadlocking us. { - auto lock = _terminal->LockForWriting(); + const auto lock = _terminal->LockForWriting(); _lastHoveredId = newId; _lastHoveredInterval = newInterval; @@ -785,16 +791,15 @@ namespace winrt::Microsoft::Terminal::Control::implementation winrt::hstring ControlCore::GetHyperlink(const Core::Point pos) const { - // Lock for the duration of our reads. - auto lock = _terminal->LockForReading(); + const auto lock = _terminal->LockForReading(); return winrt::hstring{ _terminal->GetHyperlinkAtViewportPosition(til::point{ pos }) }; } winrt::hstring ControlCore::HoveredUriText() const { - auto lock = _terminal->LockForReading(); // Lock for the duration of our reads. if (_lastHoveredCell.has_value()) { + const auto lock = _terminal->LockForReading(); auto uri{ _terminal->GetHyperlinkAtViewportPosition(*_lastHoveredCell) }; uri.resize(std::min(1024u, uri.size())); // Truncate for display return winrt::hstring{ uri }; @@ -814,7 +819,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation { _settings = winrt::make_self(settings, newAppearance); - auto lock = _terminal->LockForWriting(); + const auto lock = _terminal->LockForWriting(); _cellWidth = CSSLengthPercentage::FromString(_settings->CellWidth().c_str()); _cellHeight = CSSLengthPercentage::FromString(_settings->CellHeight().c_str()); @@ -856,7 +861,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation // - INVARIANT: This method can only be called if the caller DOES NOT HAVE writing lock on the terminal. void ControlCore::ApplyAppearance(const bool& focused) { - auto lock = _terminal->LockForWriting(); + const auto lock = _terminal->LockForWriting(); const auto& newAppearance{ focused ? _settings->FocusedAppearance() : _settings->UnfocusedAppearance() }; // Update the terminal core with its new Core settings _terminal->UpdateAppearance(*newAppearance); @@ -1101,7 +1106,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation _panelHeight = height; _compositionScale = scale; - auto lock = _terminal->LockForWriting(); + const auto lock = _terminal->LockForWriting(); if (scaleChanged) { // _updateFont relies on the new _compositionScale set above @@ -1112,7 +1117,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation void ControlCore::SetSelectionAnchor(const til::point position) { - auto lock = _terminal->LockForWriting(); + const auto lock = _terminal->LockForWriting(); _terminal->SetSelectionAnchor(position); } @@ -1123,7 +1128,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation // to throw it all in a struct and pass it along. Control::SelectionData ControlCore::SelectionInfo() const { - auto lock = _terminal->LockForReading(); + const auto lock = _terminal->LockForReading(); Control::SelectionData info; const auto start{ _terminal->SelectionStartForRendering() }; @@ -1146,15 +1151,13 @@ namespace winrt::Microsoft::Terminal::Control::implementation // - position: the point in terminal coordinates (in cells, not pixels) void ControlCore::SetEndSelectionPoint(const til::point position) { + const auto lock = _terminal->LockForWriting(); + if (!_terminal->IsSelectionActive()) { return; } - // Have to take the lock because the renderer will not draw correctly if - // you move its endpoints while it is generating a frame. - auto lock = _terminal->LockForWriting(); - til::point terminalPosition{ std::clamp(position.x, 0, _terminal->GetViewport().Width() - 1), std::clamp(position.y, 0, _terminal->GetViewport().Height() - 1) @@ -1182,6 +1185,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation bool ControlCore::CopySelectionToClipboard(bool singleLine, const Windows::Foundation::IReference& formats) { + const auto lock = _terminal->LockForWriting(); + // no selection --> nothing to copy if (!_terminal->IsSelectionActive()) { @@ -1231,21 +1236,21 @@ namespace winrt::Microsoft::Terminal::Control::implementation void ControlCore::SelectAll() { - auto lock = _terminal->LockForWriting(); + const auto lock = _terminal->LockForWriting(); _terminal->SelectAll(); _updateSelectionUI(); } void ControlCore::ClearSelection() { - auto lock = _terminal->LockForWriting(); + const auto lock = _terminal->LockForWriting(); _terminal->ClearSelection(); _updateSelectionUI(); } bool ControlCore::ToggleBlockSelection() { - auto lock = _terminal->LockForWriting(); + const auto lock = _terminal->LockForWriting(); if (_terminal->IsSelectionActive()) { _terminal->SetBlockSelection(!_terminal->IsBlockSelection()); @@ -1260,7 +1265,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation void ControlCore::ToggleMarkMode() { - auto lock = _terminal->LockForWriting(); + const auto lock = _terminal->LockForWriting(); _terminal->ToggleMarkMode(); _updateSelectionUI(); } @@ -1272,6 +1277,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation bool ControlCore::SwitchSelectionEndpoint() { + const auto lock = _terminal->LockForWriting(); if (_terminal->IsSelectionActive()) { _terminal->SwitchSelectionEndpoint(); @@ -1283,6 +1289,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation bool ControlCore::ExpandSelectionToWord() { + const auto lock = _terminal->LockForWriting(); if (_terminal->IsSelectionActive()) { _terminal->ExpandSelectionToWord(); @@ -1297,6 +1304,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation // before sending it over the terminal's connection. void ControlCore::PasteText(const winrt::hstring& hstr) { + const auto lock = _terminal->LockForWriting(); _terminal->WritePastedText(hstr); _terminal->ClearSelection(); _updateSelectionUI(); @@ -1346,21 +1354,25 @@ namespace winrt::Microsoft::Terminal::Control::implementation hstring ControlCore::Title() { + const auto lock = _terminal->LockForReading(); return hstring{ _terminal->GetConsoleTitle() }; } hstring ControlCore::WorkingDirectory() const { + const auto lock = _terminal->LockForReading(); return hstring{ _terminal->GetWorkingDirectory() }; } bool ControlCore::BracketedPasteEnabled() const noexcept { + const auto lock = _terminal->LockForReading(); return _terminal->IsXtermBracketedPasteModeEnabled(); } Windows::Foundation::IReference ControlCore::TabColor() noexcept { + const auto lock = _terminal->LockForReading(); auto coreColor = _terminal->GetTabColor(); return coreColor.has_value() ? Windows::Foundation::IReference{ static_cast(coreColor.value()) } : nullptr; @@ -1373,6 +1385,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation til::color ControlCore::BackgroundColor() const { + const auto lock = _terminal->LockForReading(); return _terminal->GetRenderSettings().GetColorAlias(ColorAlias::DefaultBackground); } @@ -1382,6 +1395,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation // - The taskbar state of this control const size_t ControlCore::TaskbarState() const noexcept { + const auto lock = _terminal->LockForReading(); return _terminal->GetTaskbarState(); } @@ -1391,11 +1405,13 @@ namespace winrt::Microsoft::Terminal::Control::implementation // - The taskbar progress of this control const size_t ControlCore::TaskbarProgress() const noexcept { + const auto lock = _terminal->LockForReading(); return _terminal->GetTaskbarProgress(); } int ControlCore::ScrollOffset() { + const auto lock = _terminal->LockForReading(); return _terminal->GetScrollOffset(); } @@ -1406,6 +1422,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation // - The height of the terminal in lines of text int ControlCore::ViewHeight() const { + const auto lock = _terminal->LockForReading(); return _terminal->GetViewport().Height(); } @@ -1416,6 +1433,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation // - The height of the terminal in lines of text int ControlCore::BufferHeight() const { + const auto lock = _terminal->LockForReading(); return _terminal->GetBufferHeight(); } @@ -1463,12 +1481,6 @@ namespace winrt::Microsoft::Terminal::Control::implementation return; } - // Clear the regex pattern tree so the renderer does not try to render them while scrolling - // We're **NOT** taking the lock here unlike _scrollbarChangeHandler because - // we are already under lock (since this usually happens as a result of writing). - // TODO GH#9617: refine locking around pattern tree - _terminal->ClearPatternTree(); - // Start the throttled update of our scrollbar. auto update{ winrt::make(viewTop, viewHeight, @@ -1527,6 +1539,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation bool ControlCore::HasSelection() const { + const auto lock = _terminal->LockForReading(); return _terminal->IsSelectionActive(); } @@ -1538,6 +1551,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation Windows::Foundation::Collections::IVector ControlCore::SelectedText(bool trimTrailingWhitespace) const { // RetrieveSelectedTextFromBuffer will lock while it's reading + const auto lock = _terminal->LockForReading(); const auto internalResult{ _terminal->RetrieveSelectedTextFromBuffer(trimTrailingWhitespace).text }; auto result = winrt::single_threaded_vector(); @@ -1565,7 +1579,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation // - void ControlCore::Search(const winrt::hstring& text, const bool goForward, const bool caseSensitive) { - auto lock = _terminal->LockForWriting(); + const auto lock = _terminal->LockForWriting(); if (_searcher.ResetIfStale(*GetRenderData(), text, !goForward, !caseSensitive)) { @@ -1690,7 +1704,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation void ControlCore::BlinkAttributeTick() { - auto lock = _terminal->LockForWriting(); + const auto lock = _terminal->LockForWriting(); auto& renderSettings = _terminal->GetRenderSettings(); renderSettings.ToggleBlinkRendition(*_renderer); @@ -1698,13 +1712,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation void ControlCore::BlinkCursor() { - if (!_terminal->IsCursorBlinkingAllowed() && - _terminal->IsCursorVisible()) - { - return; - } - // SetCursorOn will take the write lock for you. - _terminal->SetCursorOn(!_terminal->IsCursorOn()); + const auto lock = _terminal->LockForWriting(); + _terminal->BlinkCursor(); } bool ControlCore::CursorOn() const @@ -1714,22 +1723,26 @@ namespace winrt::Microsoft::Terminal::Control::implementation void ControlCore::CursorOn(const bool isCursorOn) { + const auto lock = _terminal->LockForWriting(); _terminal->SetCursorOn(isCursorOn); } void ControlCore::ResumeRendering() { + const auto lock = _terminal->LockForWriting(); _renderer->ResetErrorStateAndResume(); } bool ControlCore::IsVtMouseModeEnabled() const { - return _terminal != nullptr && _terminal->IsTrackingMouseInput(); + const auto lock = _terminal->LockForWriting(); + return _terminal->IsTrackingMouseInput(); } bool ControlCore::ShouldSendAlternateScroll(const unsigned int uiButton, const int32_t delta) const { - return _terminal != nullptr && _terminal->ShouldSendAlternateScroll(uiButton, delta); + const auto lock = _terminal->LockForWriting(); + return _terminal->ShouldSendAlternateScroll(uiButton, delta); } Core::Point ControlCore::CursorPosition() const @@ -1740,7 +1753,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation return { 0, 0 }; } - auto lock = _terminal->LockForReading(); + const auto lock = _terminal->LockForReading(); return _terminal->GetViewportRelativeCursorPosition().to_core_point(); } @@ -1756,7 +1769,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation const bool isOnOriginalPosition, bool& selectionNeedsToBeCopied) { - auto lock = _terminal->LockForWriting(); + const auto lock = _terminal->LockForWriting(); // handle ALT key _terminal->SetBlockSelection(altEnabled); @@ -1782,14 +1795,14 @@ namespace winrt::Microsoft::Terminal::Control::implementation // the selection (we need to reset selection on double-click or // triple-click, so it captures the word or the line, rather than // extending the selection) - if (HasSelection() && (!shiftEnabled || isOnOriginalPosition)) + if (_terminal->IsSelectionActive() && (!shiftEnabled || isOnOriginalPosition)) { // Reset the selection _terminal->ClearSelection(); selectionNeedsToBeCopied = false; // there's no selection, so there's nothing to update } - if (shiftEnabled && HasSelection()) + if (shiftEnabled && _terminal->IsSelectionActive()) { // If shift is pressed and there is a selection we extend it using // the selection mode (expand the "end" selection point) @@ -1878,6 +1891,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation // - Updates the renderer's representation of the selection as well as the selection marker overlay in TermControl void ControlCore::_updateSelectionUI() { + const auto lock = _terminal->LockForWriting(); _renderer->TriggerSelection(); // only show the markers if we're doing a keyboard selection or in mark mode const bool showMarkers{ _terminal->SelectionMode() >= ::Microsoft::Terminal::Core::Terminal::SelectionInteractionMode::Keyboard }; @@ -1887,10 +1901,12 @@ namespace winrt::Microsoft::Terminal::Control::implementation void ControlCore::AttachUiaEngine(::Microsoft::Console::Render::IRenderEngine* const pEngine) { // _renderer will always exist since it's introduced in the ctor + const auto lock = _terminal->LockForWriting(); _renderer->AddRenderEngine(pEngine); } void ControlCore::DetachUiaEngine(::Microsoft::Console::Render::IRenderEngine* const pEngine) { + const auto lock = _terminal->LockForWriting(); _renderer->RemoveRenderEngine(pEngine); } @@ -1918,7 +1934,10 @@ namespace winrt::Microsoft::Terminal::Control::implementation { try { - _terminal->Write(hstr); + { + const auto lock = _terminal->LockForWriting(); + _terminal->Write(hstr); + } // Start the throttled update of where our hyperlinks are. const auto shared = _shared.lock_shared(); @@ -1959,6 +1978,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation { if (clearType == Control::ClearBufferType::Scrollback || clearType == Control::ClearBufferType::All) { + const auto lock = _terminal->LockForWriting(); _terminal->EraseScrollback(); } @@ -1976,7 +1996,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation hstring ControlCore::ReadEntireBuffer() const { - auto terminalLock = _terminal->LockForWriting(); + const auto lock = _terminal->LockForWriting(); const auto& textBuffer = _terminal->GetTextBuffer(); @@ -2004,7 +2024,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation // Get all of our recent commands. This will only really work if the user has enabled shell integration. Control::CommandHistoryContext ControlCore::CommandHistory() const { - auto terminalLock = _terminal->LockForWriting(); + const auto lock = _terminal->LockForWriting(); const auto& textBuffer = _terminal->GetTextBuffer(); std::vector commands; @@ -2089,6 +2109,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation } else { + const auto lock = _terminal->LockForReading(); s = _terminal->GetColorScheme(); } @@ -2112,8 +2133,6 @@ namespace winrt::Microsoft::Terminal::Control::implementation // - void ControlCore::ColorScheme(const Core::Scheme& scheme) { - auto l{ _terminal->LockForWriting() }; - _settings->FocusedAppearance()->DefaultForeground(scheme.Foreground); _settings->FocusedAppearance()->DefaultBackground(scheme.Background); _settings->FocusedAppearance()->CursorColor(scheme.CursorColor); @@ -2136,10 +2155,9 @@ namespace winrt::Microsoft::Terminal::Control::implementation _settings->FocusedAppearance()->SetColorTableEntry(14, scheme.BrightCyan); _settings->FocusedAppearance()->SetColorTableEntry(15, scheme.BrightWhite); + const auto lock = _terminal->LockForWriting(); _terminal->ApplyScheme(scheme); - _renderEngine->SetSelectionBackground(til::color{ _settings->SelectionBackground() }); - _renderer->TriggerRedrawAll(true); } @@ -2211,6 +2229,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation // focused. const auto previous = std::exchange(_isReadOnly, false); const auto restore = wil::scope_exit([&]() { _isReadOnly = previous; }); + const auto lock = _terminal->LockForWriting(); _terminal->FocusChanged(focused); } @@ -2245,7 +2264,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation Windows::Foundation::Collections::IVector ControlCore::ScrollMarks() const { - auto internalMarks{ _terminal->GetScrollMarks() }; + const auto lock = _terminal->LockForReading(); + const auto& internalMarks{ _terminal->GetScrollMarks() }; auto v = winrt::single_threaded_observable_vector(); for (const auto& mark : internalMarks) { @@ -2270,6 +2290,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation void ControlCore::AddMark(const Control::ScrollMark& mark) { + const auto lock = _terminal->LockForReading(); ::ScrollMark m{}; if (mark.Color.HasValue) @@ -2277,7 +2298,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation m.color = til::color{ mark.Color.Color }; } - if (HasSelection()) + if (_terminal->IsSelectionActive()) { m.start = til::point{ _terminal->GetSelectionAnchor() }; m.end = til::point{ _terminal->GetSelectionEnd() }; @@ -2291,11 +2312,22 @@ namespace winrt::Microsoft::Terminal::Control::implementation // set the start & end to the cursor position. _terminal->AddMark(m, m.start, m.end, true); } - void ControlCore::ClearMark() { _terminal->ClearMark(); } - void ControlCore::ClearAllMarks() { _terminal->ClearAllMarks(); } + + void ControlCore::ClearMark() + { + const auto lock = _terminal->LockForWriting(); + _terminal->ClearMark(); + } + + void ControlCore::ClearAllMarks() + { + const auto lock = _terminal->LockForWriting(); + _terminal->ClearAllMarks(); + } void ControlCore::ScrollToMark(const Control::ScrollToMarkDirection& direction) { + const auto lock = _terminal->LockForWriting(); const auto currentOffset = ScrollOffset(); const auto& marks{ _terminal->GetScrollMarks() }; @@ -2402,15 +2434,16 @@ namespace winrt::Microsoft::Terminal::Control::implementation const auto bufferSize{ _terminal->GetTextBuffer().GetSize() }; bufferSize.DecrementInBounds(s.end); - auto lock = _terminal->LockForWriting(); _terminal->SelectNewRegion(s.start, s.end); _renderer->TriggerSelection(); } void ControlCore::SelectCommand(const bool goUp) { - const til::point start = HasSelection() ? (goUp ? _terminal->GetSelectionAnchor() : _terminal->GetSelectionEnd()) : - _terminal->GetTextBuffer().GetCursor().GetPosition(); + const auto lock = _terminal->LockForWriting(); + + const til::point start = _terminal->IsSelectionActive() ? (goUp ? _terminal->GetSelectionAnchor() : _terminal->GetSelectionEnd()) : + _terminal->GetTextBuffer().GetCursor().GetPosition(); std::optional<::ScrollMark> nearest{ std::nullopt }; const auto& marks{ _terminal->GetScrollMarks() }; @@ -2449,8 +2482,10 @@ namespace winrt::Microsoft::Terminal::Control::implementation void ControlCore::SelectOutput(const bool goUp) { - const til::point start = HasSelection() ? (goUp ? _terminal->GetSelectionAnchor() : _terminal->GetSelectionEnd()) : - _terminal->GetTextBuffer().GetCursor().GetPosition(); + const auto lock = _terminal->LockForWriting(); + + const til::point start = _terminal->IsSelectionActive() ? (goUp ? _terminal->GetSelectionAnchor() : _terminal->GetSelectionEnd()) : + _terminal->GetTextBuffer().GetCursor().GetPosition(); std::optional<::ScrollMark> nearest{ std::nullopt }; const auto& marks{ _terminal->GetScrollMarks() }; @@ -2483,7 +2518,9 @@ namespace winrt::Microsoft::Terminal::Control::implementation void ControlCore::ColorSelection(const Control::SelectionColor& fg, const Control::SelectionColor& bg, Core::MatchMode matchMode) { - if (HasSelection()) + const auto lock = _terminal->LockForWriting(); + + if (_terminal->IsSelectionActive()) { const auto pForeground = winrt::get_self(fg); const auto pBackground = winrt::get_self(bg); @@ -2521,6 +2558,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation { // viewportRelativeCharacterPosition is relative to the current // viewport, so adjust for that: + const auto lock = _terminal->LockForReading(); _contextMenuBufferPosition = _terminal->GetViewport().Origin() + viewportRelativeCharacterPosition; } @@ -2529,6 +2567,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation bool (*filter)(const ::ScrollMark&), til::point_span (*getSpan)(const ::ScrollMark&)) { + const auto lock = _terminal->LockForWriting(); + // Do nothing if the caller didn't give us a way to get the span to select for this mark. if (!getSpan) { @@ -2575,6 +2615,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation const til::point& pos, bool (*filter)(const ::ScrollMark&)) { + const auto lock = _terminal->LockForWriting(); + // Don't show this if the click was on the selection if (_terminal->IsSelectionActive() && _terminal->GetSelectionAnchor() <= pos && diff --git a/src/cascadia/TerminalCore/Terminal.cpp b/src/cascadia/TerminalCore/Terminal.cpp index 40156064b3f..b2a99833587 100644 --- a/src/cascadia/TerminalCore/Terminal.cpp +++ b/src/cascadia/TerminalCore/Terminal.cpp @@ -87,15 +87,15 @@ void Terminal::UpdateSettings(ICoreSettings settings) _trimBlockSelection = settings.TrimBlockSelection(); _autoMarkPrompts = settings.AutoMarkPrompts(); - _terminalInput.ForceDisableWin32InputMode(settings.ForceVTInput()); + _getTerminalInput().ForceDisableWin32InputMode(settings.ForceVTInput()); if (settings.TabColor() == nullptr) { - _renderSettings.SetColorTableEntry(TextColor::FRAME_BACKGROUND, INVALID_COLOR); + GetRenderSettings().SetColorTableEntry(TextColor::FRAME_BACKGROUND, INVALID_COLOR); } else { - _renderSettings.SetColorTableEntry(TextColor::FRAME_BACKGROUND, til::color{ settings.TabColor().Value() }); + GetRenderSettings().SetColorTableEntry(TextColor::FRAME_BACKGROUND, til::color{ settings.TabColor().Value() }); } if (!_startingTabColor && settings.StartingTabColor()) @@ -125,35 +125,37 @@ void Terminal::UpdateSettings(ICoreSettings settings) // - appearance: an ICoreAppearance with new settings values for us to use. void Terminal::UpdateAppearance(const ICoreAppearance& appearance) { - _renderSettings.SetRenderMode(RenderSettings::Mode::IntenseIsBold, appearance.IntenseIsBold()); - _renderSettings.SetRenderMode(RenderSettings::Mode::IntenseIsBright, appearance.IntenseIsBright()); + auto& renderSettings = GetRenderSettings(); + + renderSettings.SetRenderMode(RenderSettings::Mode::IntenseIsBold, appearance.IntenseIsBold()); + renderSettings.SetRenderMode(RenderSettings::Mode::IntenseIsBright, appearance.IntenseIsBright()); switch (appearance.AdjustIndistinguishableColors()) { case AdjustTextMode::Always: - _renderSettings.SetRenderMode(RenderSettings::Mode::IndexedDistinguishableColors, false); - _renderSettings.SetRenderMode(RenderSettings::Mode::AlwaysDistinguishableColors, true); + renderSettings.SetRenderMode(RenderSettings::Mode::IndexedDistinguishableColors, false); + renderSettings.SetRenderMode(RenderSettings::Mode::AlwaysDistinguishableColors, true); break; case AdjustTextMode::Indexed: - _renderSettings.SetRenderMode(RenderSettings::Mode::IndexedDistinguishableColors, true); - _renderSettings.SetRenderMode(RenderSettings::Mode::AlwaysDistinguishableColors, false); + renderSettings.SetRenderMode(RenderSettings::Mode::IndexedDistinguishableColors, true); + renderSettings.SetRenderMode(RenderSettings::Mode::AlwaysDistinguishableColors, false); break; case AdjustTextMode::Never: - _renderSettings.SetRenderMode(RenderSettings::Mode::IndexedDistinguishableColors, false); - _renderSettings.SetRenderMode(RenderSettings::Mode::AlwaysDistinguishableColors, false); + renderSettings.SetRenderMode(RenderSettings::Mode::IndexedDistinguishableColors, false); + renderSettings.SetRenderMode(RenderSettings::Mode::AlwaysDistinguishableColors, false); break; } const til::color newBackgroundColor{ appearance.DefaultBackground() }; - _renderSettings.SetColorAlias(ColorAlias::DefaultBackground, TextColor::DEFAULT_BACKGROUND, newBackgroundColor); + renderSettings.SetColorAlias(ColorAlias::DefaultBackground, TextColor::DEFAULT_BACKGROUND, newBackgroundColor); const til::color newForegroundColor{ appearance.DefaultForeground() }; - _renderSettings.SetColorAlias(ColorAlias::DefaultForeground, TextColor::DEFAULT_FOREGROUND, newForegroundColor); + renderSettings.SetColorAlias(ColorAlias::DefaultForeground, TextColor::DEFAULT_FOREGROUND, newForegroundColor); const til::color newCursorColor{ appearance.CursorColor() }; - _renderSettings.SetColorTableEntry(TextColor::CURSOR_COLOR, newCursorColor); + renderSettings.SetColorTableEntry(TextColor::CURSOR_COLOR, newCursorColor); for (auto i = 0; i < 16; i++) { - _renderSettings.SetColorTableEntry(i, til::color{ appearance.GetColorTableEntry(i) }); + renderSettings.SetColorTableEntry(i, til::color{ appearance.GetColorTableEntry(i) }); } auto cursorShape = CursorType::VerticalBar; @@ -445,17 +447,15 @@ std::wstring_view Terminal::GetWorkingDirectory() noexcept try { _activeBuffer().TriggerRedrawAll(); + _NotifyScrollEvent(); } CATCH_LOG(); - _NotifyScrollEvent(); return S_OK; } void Terminal::Write(std::wstring_view stringView) { - auto lock = LockForWriting(); - const auto& cursor = _activeBuffer().GetCursor(); const til::point cursorPosBefore{ cursor.GetPosition() }; @@ -501,7 +501,6 @@ void Terminal::TrySnapOnInput() { if (_snapOnInput && _scrollOffset != 0) { - auto lock = LockForWriting(); _scrollOffset = 0; _NotifyScrollEvent(); } @@ -515,7 +514,7 @@ void Terminal::TrySnapOnInput() // - true, if we are tracking mouse input. False, otherwise bool Terminal::IsTrackingMouseInput() const noexcept { - return _terminalInput.IsTrackingMouseInput(); + return _getTerminalInput().IsTrackingMouseInput(); } // Routine Description: @@ -529,7 +528,7 @@ bool Terminal::IsTrackingMouseInput() const noexcept bool Terminal::ShouldSendAlternateScroll(const unsigned int uiButton, const int32_t delta) const noexcept { - return _terminalInput.ShouldSendAlternateScroll(uiButton, ::base::saturated_cast(delta)); + return _getTerminalInput().ShouldSendAlternateScroll(uiButton, ::base::saturated_cast(delta)); } // Method Description: @@ -716,7 +715,7 @@ bool Terminal::SendKeyEvent(const WORD vkey, } const auto keyEv = SynthesizeKeyEvent(keyDown, 1, vkey, sc, ch, states.Value()); - return _handleTerminalInputResult(_terminalInput.HandleKey(keyEv)); + return _handleTerminalInputResult(_getTerminalInput().HandleKey(keyEv)); } // Method Description: @@ -740,7 +739,7 @@ bool Terminal::SendMouseEvent(til::point viewportPos, const unsigned int uiButto // We shouldn't throw away perfectly good events when they're offscreen, so we just // clamp them to be within the range [(0, 0), (W, H)]. _GetMutableViewport().ToOrigin().Clamp(viewportPos); - return _handleTerminalInputResult(_terminalInput.HandleMouse(viewportPos, uiButton, GET_KEYSTATE_WPARAM(states.Value()), wheelDelta, state)); + return _handleTerminalInputResult(_getTerminalInput().HandleMouse(viewportPos, uiButton, GET_KEYSTATE_WPARAM(states.Value()), wheelDelta, state)); } // Method Description: @@ -793,7 +792,7 @@ bool Terminal::SendCharEvent(const wchar_t ch, const WORD scanCode, const Contro } const auto keyDown = SynthesizeKeyEvent(true, 1, vkey, scanCode, ch, states.Value()); - return _handleTerminalInputResult(_terminalInput.HandleKey(keyDown)); + return _handleTerminalInputResult(_getTerminalInput().HandleKey(keyDown)); } // Method Description: @@ -806,22 +805,21 @@ bool Terminal::SendCharEvent(const wchar_t ch, const WORD scanCode, const Contro // - none void Terminal::FocusChanged(const bool focused) { - _handleTerminalInputResult(_terminalInput.HandleFocus(focused)); + _handleTerminalInputResult(_getTerminalInput().HandleFocus(focused)); } // Method Description: // - Invalidates the regions described in the given pattern tree for the rendering purposes // Arguments: // - The interval tree containing regions that need to be invalidated -void Terminal::_InvalidatePatternTree(const interval_tree::IntervalTree& tree) +void Terminal::_InvalidatePatternTree() { const auto vis = _VisibleStartIndex(); - auto invalidate = [=](const PointTree::interval& interval) { + _patternIntervalTree.visit_all([&](const PointTree::interval& interval) { const til::point startCoord{ interval.start.x, interval.start.y + vis }; const til::point endCoord{ interval.stop.x, interval.stop.y + vis }; _InvalidateFromCoords(startCoord, endCoord); - }; - tree.visit_all(invalidate); + }); } // Method Description: @@ -978,14 +976,29 @@ WORD Terminal::_TakeVirtualKeyFromLastKeyEvent(const WORD scanCode) noexcept return codes.ScanCode == scanCode ? codes.VirtualKey : 0; } +void Terminal::_assertLocked() const noexcept +{ +#ifndef NDEBUG + if (!_readWriteLock.is_locked()) + { + // __debugbreak() has the benefit over assert() that the debugger jumps right here to this line. + // That way there's no need to first click any dialogues, etc. The disadvantage of course is that the + // application just crashes if no debugger is attached. But hey, that's a great incentive to fix the bug! + __debugbreak(); + } +#endif +} + // Method Description: // - Acquire a read lock on the terminal. // Return Value: // - a shared_lock which can be used to unlock the terminal. The shared_lock // will release this lock when it's destructed. -[[nodiscard]] std::unique_lock Terminal::LockForReading() +[[nodiscard]] std::unique_lock Terminal::LockForReading() const noexcept { - return std::unique_lock{ _readWriteLock }; +#pragma warning(suppress : 26447) // The function is declared 'noexcept' but calls function 'recursive_ticket_lock>()' which may throw exceptions (f.6). +#pragma warning(suppress : 26492) // Don't use const_cast to cast away const or volatile + return std::unique_lock{ const_cast(_readWriteLock) }; } // Method Description: @@ -993,8 +1006,9 @@ WORD Terminal::_TakeVirtualKeyFromLastKeyEvent(const WORD scanCode) noexcept // Return Value: // - a unique_lock which can be used to unlock the terminal. The unique_lock // will release this lock when it's destructed. -[[nodiscard]] std::unique_lock Terminal::LockForWriting() +[[nodiscard]] std::unique_lock Terminal::LockForWriting() noexcept { +#pragma warning(suppress : 26447) // The function is declared 'noexcept' but calls function 'recursive_ticket_lock>()' which may throw exceptions (f.6). return std::unique_lock{ _readWriteLock }; } @@ -1032,6 +1046,30 @@ int Terminal::ViewEndIndex() const noexcept return _inAltBuffer() ? _altBufferSize.height - 1 : _mutableViewport.BottomInclusive(); } +RenderSettings& Terminal::GetRenderSettings() noexcept +{ + _assertLocked(); + return _renderSettings; +} + +const RenderSettings& Terminal::GetRenderSettings() const noexcept +{ + _assertLocked(); + return _renderSettings; +} + +TerminalInput& Terminal::_getTerminalInput() noexcept +{ + _assertLocked(); + return _terminalInput; +} + +const TerminalInput& Terminal::_getTerminalInput() const noexcept +{ + _assertLocked(); + return _terminalInput; +} + // _VisibleStartIndex is the first visible line of the buffer int Terminal::_VisibleStartIndex() const noexcept { @@ -1072,14 +1110,14 @@ void Terminal::_PreserveUserScrollOffset(const int viewportDelta) noexcept void Terminal::UserScrollViewport(const int viewTop) { + // Clear the regex pattern tree so the renderer does not try to render them while scrolling + _clearPatternTree(); + if (_inAltBuffer()) { return; } - // we're going to modify state here that the renderer could be reading. - auto lock = LockForWriting(); - const auto clampedNewTop = std::max(0, viewTop); const auto realTop = ViewStartIndex(); const auto newDelta = realTop - clampedNewTop; @@ -1098,9 +1136,11 @@ int Terminal::GetScrollOffset() noexcept return _VisibleStartIndex(); } -void Terminal::_NotifyScrollEvent() noexcept -try +void Terminal::_NotifyScrollEvent() { + // See UserScrollViewport(). + _clearPatternTree(); + if (_pfnScrollPositionChanged) { const auto visible = _GetVisibleViewport(); @@ -1110,7 +1150,6 @@ try _pfnScrollPositionChanged(top, height, bottom); } } -CATCH_LOG() void Terminal::_NotifyTerminalCursorPositionChanged() noexcept { @@ -1182,6 +1221,18 @@ void Terminal::SetPlayMidiNoteCallback(std::function Terminal::GetTabColor() const } else { - const auto tabColor = _renderSettings.GetColorAlias(ColorAlias::FrameBackground); + const auto tabColor = GetRenderSettings().GetColorAlias(ColorAlias::FrameBackground); return tabColor == INVALID_COLOR ? std::nullopt : std::optional{ tabColor }; } } @@ -1266,56 +1313,59 @@ void Microsoft::Terminal::Core::Terminal::CompletionsChangedCallback(std::functi Scheme Terminal::GetColorScheme() const { - Scheme s; + const auto& renderSettings = GetRenderSettings(); - s.Foreground = til::color{ _renderSettings.GetColorAlias(ColorAlias::DefaultForeground) }; - s.Background = til::color{ _renderSettings.GetColorAlias(ColorAlias::DefaultBackground) }; + Scheme s; + s.Foreground = til::color{ renderSettings.GetColorAlias(ColorAlias::DefaultForeground) }; + s.Background = til::color{ renderSettings.GetColorAlias(ColorAlias::DefaultBackground) }; // SelectionBackground is stored in the ControlAppearance - s.CursorColor = til::color{ _renderSettings.GetColorTableEntry(TextColor::CURSOR_COLOR) }; - - s.Black = til::color{ _renderSettings.GetColorTableEntry(TextColor::DARK_BLACK) }; - s.Red = til::color{ _renderSettings.GetColorTableEntry(TextColor::DARK_RED) }; - s.Green = til::color{ _renderSettings.GetColorTableEntry(TextColor::DARK_GREEN) }; - s.Yellow = til::color{ _renderSettings.GetColorTableEntry(TextColor::DARK_YELLOW) }; - s.Blue = til::color{ _renderSettings.GetColorTableEntry(TextColor::DARK_BLUE) }; - s.Purple = til::color{ _renderSettings.GetColorTableEntry(TextColor::DARK_MAGENTA) }; - s.Cyan = til::color{ _renderSettings.GetColorTableEntry(TextColor::DARK_CYAN) }; - s.White = til::color{ _renderSettings.GetColorTableEntry(TextColor::DARK_WHITE) }; - s.BrightBlack = til::color{ _renderSettings.GetColorTableEntry(TextColor::BRIGHT_BLACK) }; - s.BrightRed = til::color{ _renderSettings.GetColorTableEntry(TextColor::BRIGHT_RED) }; - s.BrightGreen = til::color{ _renderSettings.GetColorTableEntry(TextColor::BRIGHT_GREEN) }; - s.BrightYellow = til::color{ _renderSettings.GetColorTableEntry(TextColor::BRIGHT_YELLOW) }; - s.BrightBlue = til::color{ _renderSettings.GetColorTableEntry(TextColor::BRIGHT_BLUE) }; - s.BrightPurple = til::color{ _renderSettings.GetColorTableEntry(TextColor::BRIGHT_MAGENTA) }; - s.BrightCyan = til::color{ _renderSettings.GetColorTableEntry(TextColor::BRIGHT_CYAN) }; - s.BrightWhite = til::color{ _renderSettings.GetColorTableEntry(TextColor::BRIGHT_WHITE) }; + s.CursorColor = til::color{ renderSettings.GetColorTableEntry(TextColor::CURSOR_COLOR) }; + + s.Black = til::color{ renderSettings.GetColorTableEntry(TextColor::DARK_BLACK) }; + s.Red = til::color{ renderSettings.GetColorTableEntry(TextColor::DARK_RED) }; + s.Green = til::color{ renderSettings.GetColorTableEntry(TextColor::DARK_GREEN) }; + s.Yellow = til::color{ renderSettings.GetColorTableEntry(TextColor::DARK_YELLOW) }; + s.Blue = til::color{ renderSettings.GetColorTableEntry(TextColor::DARK_BLUE) }; + s.Purple = til::color{ renderSettings.GetColorTableEntry(TextColor::DARK_MAGENTA) }; + s.Cyan = til::color{ renderSettings.GetColorTableEntry(TextColor::DARK_CYAN) }; + s.White = til::color{ renderSettings.GetColorTableEntry(TextColor::DARK_WHITE) }; + s.BrightBlack = til::color{ renderSettings.GetColorTableEntry(TextColor::BRIGHT_BLACK) }; + s.BrightRed = til::color{ renderSettings.GetColorTableEntry(TextColor::BRIGHT_RED) }; + s.BrightGreen = til::color{ renderSettings.GetColorTableEntry(TextColor::BRIGHT_GREEN) }; + s.BrightYellow = til::color{ renderSettings.GetColorTableEntry(TextColor::BRIGHT_YELLOW) }; + s.BrightBlue = til::color{ renderSettings.GetColorTableEntry(TextColor::BRIGHT_BLUE) }; + s.BrightPurple = til::color{ renderSettings.GetColorTableEntry(TextColor::BRIGHT_MAGENTA) }; + s.BrightCyan = til::color{ renderSettings.GetColorTableEntry(TextColor::BRIGHT_CYAN) }; + s.BrightWhite = til::color{ renderSettings.GetColorTableEntry(TextColor::BRIGHT_WHITE) }; return s; } void Terminal::ApplyScheme(const Scheme& colorScheme) { - _renderSettings.SetColorAlias(ColorAlias::DefaultForeground, TextColor::DEFAULT_FOREGROUND, til::color{ colorScheme.Foreground }); - _renderSettings.SetColorAlias(ColorAlias::DefaultBackground, TextColor::DEFAULT_BACKGROUND, til::color{ colorScheme.Background }); - - _renderSettings.SetColorTableEntry(TextColor::DARK_BLACK, til::color{ colorScheme.Black }); - _renderSettings.SetColorTableEntry(TextColor::DARK_RED, til::color{ colorScheme.Red }); - _renderSettings.SetColorTableEntry(TextColor::DARK_GREEN, til::color{ colorScheme.Green }); - _renderSettings.SetColorTableEntry(TextColor::DARK_YELLOW, til::color{ colorScheme.Yellow }); - _renderSettings.SetColorTableEntry(TextColor::DARK_BLUE, til::color{ colorScheme.Blue }); - _renderSettings.SetColorTableEntry(TextColor::DARK_MAGENTA, til::color{ colorScheme.Purple }); - _renderSettings.SetColorTableEntry(TextColor::DARK_CYAN, til::color{ colorScheme.Cyan }); - _renderSettings.SetColorTableEntry(TextColor::DARK_WHITE, til::color{ colorScheme.White }); - _renderSettings.SetColorTableEntry(TextColor::BRIGHT_BLACK, til::color{ colorScheme.BrightBlack }); - _renderSettings.SetColorTableEntry(TextColor::BRIGHT_RED, til::color{ colorScheme.BrightRed }); - _renderSettings.SetColorTableEntry(TextColor::BRIGHT_GREEN, til::color{ colorScheme.BrightGreen }); - _renderSettings.SetColorTableEntry(TextColor::BRIGHT_YELLOW, til::color{ colorScheme.BrightYellow }); - _renderSettings.SetColorTableEntry(TextColor::BRIGHT_BLUE, til::color{ colorScheme.BrightBlue }); - _renderSettings.SetColorTableEntry(TextColor::BRIGHT_MAGENTA, til::color{ colorScheme.BrightPurple }); - _renderSettings.SetColorTableEntry(TextColor::BRIGHT_CYAN, til::color{ colorScheme.BrightCyan }); - _renderSettings.SetColorTableEntry(TextColor::BRIGHT_WHITE, til::color{ colorScheme.BrightWhite }); - - _renderSettings.SetColorTableEntry(TextColor::CURSOR_COLOR, til::color{ colorScheme.CursorColor }); + auto& renderSettings = GetRenderSettings(); + + renderSettings.SetColorAlias(ColorAlias::DefaultForeground, TextColor::DEFAULT_FOREGROUND, til::color{ colorScheme.Foreground }); + renderSettings.SetColorAlias(ColorAlias::DefaultBackground, TextColor::DEFAULT_BACKGROUND, til::color{ colorScheme.Background }); + + renderSettings.SetColorTableEntry(TextColor::DARK_BLACK, til::color{ colorScheme.Black }); + renderSettings.SetColorTableEntry(TextColor::DARK_RED, til::color{ colorScheme.Red }); + renderSettings.SetColorTableEntry(TextColor::DARK_GREEN, til::color{ colorScheme.Green }); + renderSettings.SetColorTableEntry(TextColor::DARK_YELLOW, til::color{ colorScheme.Yellow }); + renderSettings.SetColorTableEntry(TextColor::DARK_BLUE, til::color{ colorScheme.Blue }); + renderSettings.SetColorTableEntry(TextColor::DARK_MAGENTA, til::color{ colorScheme.Purple }); + renderSettings.SetColorTableEntry(TextColor::DARK_CYAN, til::color{ colorScheme.Cyan }); + renderSettings.SetColorTableEntry(TextColor::DARK_WHITE, til::color{ colorScheme.White }); + renderSettings.SetColorTableEntry(TextColor::BRIGHT_BLACK, til::color{ colorScheme.BrightBlack }); + renderSettings.SetColorTableEntry(TextColor::BRIGHT_RED, til::color{ colorScheme.BrightRed }); + renderSettings.SetColorTableEntry(TextColor::BRIGHT_GREEN, til::color{ colorScheme.BrightGreen }); + renderSettings.SetColorTableEntry(TextColor::BRIGHT_YELLOW, til::color{ colorScheme.BrightYellow }); + renderSettings.SetColorTableEntry(TextColor::BRIGHT_BLUE, til::color{ colorScheme.BrightBlue }); + renderSettings.SetColorTableEntry(TextColor::BRIGHT_MAGENTA, til::color{ colorScheme.BrightPurple }); + renderSettings.SetColorTableEntry(TextColor::BRIGHT_CYAN, til::color{ colorScheme.BrightCyan }); + renderSettings.SetColorTableEntry(TextColor::BRIGHT_WHITE, til::color{ colorScheme.BrightWhite }); + + renderSettings.SetColorTableEntry(TextColor::CURSOR_COLOR, til::color{ colorScheme.CursorColor }); // Tell the control that the scrollbar has somehow changed. Used as a // workaround to force the control to redraw any scrollbar marks whose color @@ -1325,6 +1375,7 @@ void Terminal::ApplyScheme(const Scheme& colorScheme) bool Terminal::_inAltBuffer() const noexcept { + _assertLocked(); return _altBuffer != nullptr; } @@ -1341,7 +1392,7 @@ void Terminal::_updateUrlDetection() } else { - ClearPatternTree(); + _clearPatternTree(); } } @@ -1497,7 +1548,7 @@ void Terminal::ClearMark() // workaround to force the control to redraw any scrollbar marks _NotifyScrollEvent(); } -void Terminal::ClearAllMarks() noexcept +void Terminal::ClearAllMarks() { _activeBuffer().ClearAllMarks(); // Tell the control that the scrollbar has somehow changed. Used as a @@ -1521,28 +1572,30 @@ til::color Terminal::GetColorForMark(const ScrollMark& mark) const return *mark.color; } + const auto& renderSettings = GetRenderSettings(); + switch (mark.category) { case MarkCategory::Prompt: { - return _renderSettings.GetColorAlias(ColorAlias::DefaultForeground); + return renderSettings.GetColorAlias(ColorAlias::DefaultForeground); } case MarkCategory::Error: { - return _renderSettings.GetColorTableEntry(TextColor::BRIGHT_RED); + return renderSettings.GetColorTableEntry(TextColor::BRIGHT_RED); } case MarkCategory::Warning: { - return _renderSettings.GetColorTableEntry(TextColor::BRIGHT_YELLOW); + return renderSettings.GetColorTableEntry(TextColor::BRIGHT_YELLOW); } case MarkCategory::Success: { - return _renderSettings.GetColorTableEntry(TextColor::BRIGHT_GREEN); + return renderSettings.GetColorTableEntry(TextColor::BRIGHT_GREEN); } default: case MarkCategory::Info: { - return _renderSettings.GetColorAlias(ColorAlias::DefaultForeground); + return renderSettings.GetColorAlias(ColorAlias::DefaultForeground); } } } diff --git a/src/cascadia/TerminalCore/Terminal.hpp b/src/cascadia/TerminalCore/Terminal.hpp index 9c83710c3b5..72c34d2f629 100644 --- a/src/cascadia/TerminalCore/Terminal.hpp +++ b/src/cascadia/TerminalCore/Terminal.hpp @@ -101,8 +101,8 @@ class Microsoft::Terminal::Core::Terminal final : // WritePastedText comes from our input and goes back to the PTY's input channel void WritePastedText(std::wstring_view stringView); - [[nodiscard]] std::unique_lock LockForReading(); - [[nodiscard]] std::unique_lock LockForWriting(); + [[nodiscard]] std::unique_lock LockForReading() const noexcept; + [[nodiscard]] std::unique_lock LockForWriting() noexcept; til::recursive_ticket_lock_suspension SuspendLock() noexcept; til::CoordType GetBufferHeight() const noexcept; @@ -110,8 +110,8 @@ class Microsoft::Terminal::Core::Terminal final : int ViewStartIndex() const noexcept; int ViewEndIndex() const noexcept; - RenderSettings& GetRenderSettings() noexcept { return _renderSettings; }; - const RenderSettings& GetRenderSettings() const noexcept { return _renderSettings; }; + RenderSettings& GetRenderSettings() noexcept; + const RenderSettings& GetRenderSettings() const noexcept; const std::vector& GetScrollMarks() const noexcept; void AddMark(const ScrollMark& mark, @@ -162,7 +162,7 @@ class Microsoft::Terminal::Core::Terminal final : #pragma endregion void ClearMark(); - void ClearAllMarks() noexcept; + void ClearAllMarks(); til::color GetColorForMark(const ScrollMark& mark) const; #pragma region ITerminalInput @@ -233,11 +233,10 @@ class Microsoft::Terminal::Core::Terminal final : void SetPlayMidiNoteCallback(std::function pfn) noexcept; void CompletionsChangedCallback(std::function pfn) noexcept; - void SetCursorOn(const bool isOn); - bool IsCursorBlinkingAllowed() const noexcept; + void BlinkCursor() noexcept; + void SetCursorOn(const bool isOn) noexcept; void UpdatePatternsUnderLock(); - void ClearPatternTree(); const std::optional GetTabColor() const; @@ -406,7 +405,8 @@ class Microsoft::Terminal::Core::Terminal final : // Either way, we should make this behavior controlled by a setting. interval_tree::IntervalTree _patternIntervalTree; - void _InvalidatePatternTree(const interval_tree::IntervalTree& tree); + void _clearPatternTree(); + void _InvalidatePatternTree(); void _InvalidateFromCoords(const til::point start, const til::point end); // Since virtual keys are non-zero, you assume that this field is empty/invalid if it is. @@ -435,6 +435,10 @@ class Microsoft::Terminal::Core::Terminal final : void _StoreKeyEvent(const WORD vkey, const WORD scanCode) noexcept; WORD _TakeVirtualKeyFromLastKeyEvent(const WORD scanCode) noexcept; + void _assertLocked() const noexcept; + Console::VirtualTerminal::TerminalInput& _getTerminalInput() noexcept; + const Console::VirtualTerminal::TerminalInput& _getTerminalInput() const noexcept; + int _VisibleStartIndex() const noexcept; int _VisibleEndIndex() const noexcept; @@ -443,7 +447,7 @@ class Microsoft::Terminal::Core::Terminal final : void _PreserveUserScrollOffset(const int viewportDelta) noexcept; - void _NotifyScrollEvent() noexcept; + void _NotifyScrollEvent(); void _NotifyTerminalCursorPositionChanged() noexcept; diff --git a/src/cascadia/TerminalCore/TerminalApi.cpp b/src/cascadia/TerminalCore/TerminalApi.cpp index 6815876b270..0f4b1407031 100644 --- a/src/cascadia/TerminalCore/TerminalApi.cpp +++ b/src/cascadia/TerminalCore/TerminalApi.cpp @@ -44,6 +44,7 @@ til::rect Terminal::GetViewport() const noexcept } void Terminal::SetViewportPosition(const til::point position) noexcept +try { // The viewport is fixed at 0,0 for the alt buffer, so this is a no-op. if (!_inAltBuffer()) @@ -55,6 +56,7 @@ void Terminal::SetViewportPosition(const til::point position) noexcept _NotifyScrollEvent(); } } +CATCH_LOG() void Terminal::SetTextAttributes(const TextAttribute& attrs) noexcept { @@ -63,11 +65,13 @@ void Terminal::SetTextAttributes(const TextAttribute& attrs) noexcept void Terminal::SetSystemMode(const Mode mode, const bool enabled) noexcept { + _assertLocked(); _systemMode.set(mode, enabled); } bool Terminal::GetSystemMode(const Mode mode) const noexcept { + _assertLocked(); return _systemMode.test(mode); } @@ -78,6 +82,7 @@ void Terminal::WarningBell() void Terminal::SetWindowTitle(const std::wstring_view title) { + _assertLocked(); if (!_suppressApplicationTitle) { _title.emplace(title); @@ -87,6 +92,7 @@ void Terminal::SetWindowTitle(const std::wstring_view title) CursorType Terminal::GetUserDefaultCursorStyle() const noexcept { + _assertLocked(); return _defaultCursorShape; } @@ -121,6 +127,8 @@ void Terminal::CopyToClipboard(std::wstring_view content) // - void Terminal::SetTaskbarProgress(const ::Microsoft::Console::VirtualTerminal::DispatchTypes::TaskbarState state, const size_t progress) { + _assertLocked(); + _taskbarState = static_cast(state); switch (state) @@ -164,6 +172,8 @@ void Terminal::SetTaskbarProgress(const ::Microsoft::Console::VirtualTerminal::D void Terminal::SetWorkingDirectory(std::wstring_view uri) { + _assertLocked(); + static bool logged = false; if (!logged) { @@ -187,6 +197,8 @@ void Terminal::PlayMidiNote(const int noteNumber, const int velocity, const std: void Terminal::UseAlternateScreenBuffer(const TextAttribute& attrs) { + _assertLocked(); + // the new alt buffer is exactly the size of the viewport. _altBufferSize = _mutableViewport.Dimensions(); @@ -222,7 +234,7 @@ void Terminal::UseAlternateScreenBuffer(const TextAttribute& attrs) // GH#3321: Make sure we let the TerminalInput know that we switched // buffers. This might affect how we interpret certain mouse events. - _terminalInput.UseAlternateScreenBuffer(); + _getTerminalInput().UseAlternateScreenBuffer(); // Update scrollbars _NotifyScrollEvent(); @@ -275,7 +287,7 @@ void Terminal::UseMainScreenBuffer() // GH#3321: Make sure we let the TerminalInput know that we switched // buffers. This might affect how we interpret certain mouse events. - _terminalInput.UseMainScreenBuffer(); + _getTerminalInput().UseMainScreenBuffer(); // Update scrollbars _NotifyScrollEvent(); @@ -291,6 +303,8 @@ void Terminal::UseMainScreenBuffer() // NOTE: This is the version of AddMark that comes from VT void Terminal::MarkPrompt(const ScrollMark& mark) { + _assertLocked(); + static bool logged = false; if (!logged) { @@ -315,6 +329,8 @@ void Terminal::MarkPrompt(const ScrollMark& mark) void Terminal::MarkCommandStart() { + _assertLocked(); + const til::point cursorPos{ _activeBuffer().GetCursor().GetPosition() }; if ((_currentPromptState == PromptState::Prompt) && @@ -341,6 +357,8 @@ void Terminal::MarkCommandStart() void Terminal::MarkOutputStart() { + _assertLocked(); + const til::point cursorPos{ _activeBuffer().GetCursor().GetPosition() }; if ((_currentPromptState == PromptState::Command) && @@ -367,6 +385,8 @@ void Terminal::MarkOutputStart() void Terminal::MarkCommandFinish(std::optional error) { + _assertLocked(); + const til::point cursorPos{ _activeBuffer().GetCursor().GetPosition() }; auto category = MarkCategory::Prompt; if (error.has_value()) diff --git a/src/cascadia/TerminalCore/TerminalSelection.cpp b/src/cascadia/TerminalCore/TerminalSelection.cpp index 5f89c533896..1978f5738bc 100644 --- a/src/cascadia/TerminalCore/TerminalSelection.cpp +++ b/src/cascadia/TerminalCore/TerminalSelection.cpp @@ -92,6 +92,7 @@ std::vector Terminal::_GetSelectionSpans() const noexcept // - None const til::point Terminal::GetSelectionAnchor() const noexcept { + _assertLocked(); return _selection->start; } @@ -103,6 +104,7 @@ const til::point Terminal::GetSelectionAnchor() const noexcept // - None const til::point Terminal::GetSelectionEnd() const noexcept { + _assertLocked(); return _selection->end; } @@ -155,11 +157,13 @@ const Terminal::SelectionEndpoint Terminal::SelectionEndpointTarget() const noex // - bool representing if selection is active. Used to decide copy/paste on right click const bool Terminal::IsSelectionActive() const noexcept { + _assertLocked(); return _selection.has_value(); } const bool Terminal::IsBlockSelection() const noexcept { + _assertLocked(); return _blockSelection; } @@ -188,6 +192,8 @@ void Terminal::MultiClickSelection(const til::point viewportPos, SelectionExpans // - position: the (x,y) coordinate on the visible viewport void Terminal::SetSelectionAnchor(const til::point viewportPos) { + _assertLocked(); + _selection = SelectionAnchors{}; _selection->pivot = _ConvertToBufferCell(viewportPos); @@ -205,7 +211,7 @@ void Terminal::SetSelectionAnchor(const til::point viewportPos) // - newExpansionMode: overwrites the _multiClickSelectionMode for this function call. Used for ShiftClick void Terminal::SetSelectionEnd(const til::point viewportPos, std::optional newExpansionMode) { - if (!_selection.has_value()) + if (!IsSelectionActive()) { // capture a log for spurious endpoint sets without an active selection LOG_HR(E_ILLEGAL_STATE_CHANGE); @@ -817,6 +823,7 @@ void Terminal::_MoveByBuffer(SelectionDirection direction, til::point& pos) noex #pragma warning(disable : 26440) // changing this to noexcept would require a change to ConHost's selection model void Terminal::ClearSelection() { + _assertLocked(); _selection = std::nullopt; _selectionMode = SelectionInteractionMode::None; _selectionIsTargetingUrl = false; @@ -832,8 +839,6 @@ void Terminal::ClearSelection() // - wstring text from buffer. If extended to multiple lines, each line is separated by \r\n const TextBuffer::TextAndColor Terminal::RetrieveSelectedTextFromBuffer(bool singleLine) { - auto lock = LockForReading(); - const auto selectionRects = _GetSelectionRects(); const auto GetAttributeColors = [&](const auto& attr) { diff --git a/src/cascadia/TerminalCore/terminalrenderdata.cpp b/src/cascadia/TerminalCore/terminalrenderdata.cpp index 69ea72d764f..034a65ab784 100644 --- a/src/cascadia/TerminalCore/terminalrenderdata.cpp +++ b/src/cascadia/TerminalCore/terminalrenderdata.cpp @@ -29,11 +29,13 @@ const TextBuffer& Terminal::GetTextBuffer() const noexcept const FontInfo& Terminal::GetFontInfo() const noexcept { + _assertLocked(); return _fontInfo; } void Terminal::SetFontInfo(const FontInfo& fontInfo) { + _assertLocked(); _fontInfo = fontInfo; } @@ -105,6 +107,8 @@ const std::wstring Microsoft::Terminal::Core::Terminal::GetHyperlinkCustomId(uin // - The pattern IDs of the location const std::vector Terminal::GetPatternId(const til::point location) const { + _assertLocked(); + // Look through our interval tree for this location const auto intervals = _patternIntervalTree.findOverlapping({ location.x + 1, location.y }, location); if (intervals.size() == 0) @@ -125,7 +129,7 @@ const std::vector Terminal::GetPatternId(const til::point location) cons std::pair Terminal::GetAttributeColors(const TextAttribute& attr) const noexcept { - return _renderSettings.GetAttributeColors(attr); + return GetRenderSettings().GetAttributeColors(attr); } std::vector Terminal::GetSelectionRects() noexcept @@ -185,19 +189,14 @@ void Terminal::SelectNewRegion(const til::point coordStart, const til::point coo } const std::wstring_view Terminal::GetConsoleTitle() const noexcept -try { + _assertLocked(); if (_title.has_value()) { - return _title.value(); + return *_title; } return _startingTitle; } -catch (...) -{ - LOG_CAUGHT_EXCEPTION(); - return {}; -} // Method Description: // - Lock the terminal for reading the contents of the buffer. Ensures that the @@ -223,5 +222,6 @@ const bool Terminal::IsUiaDataInitialized() const noexcept // when a screen reader requests it. However, the terminal might not be fully // initialized yet. So we use this to check if any crucial components of // UiaData are not yet initialized. + _assertLocked(); return !!_mainBuffer; }