mirror of
https://github.com/microsoft/terminal.git
synced 2025-12-11 04:38:24 -06:00
Fix a shutdown race condition in ControlCore (#18632)
I found multiple issues while investigating this: * Render thread shutdown is racy, because it doesn't actually stop the render thread. * Lifetime management in `ControlCore` failed to account for the circular dependency of render thread --> renderer --> render data --> terminal --> renderer --> render thread. Fixed by reordering the `ControlCore` members to ensure their correct destruction. * Ensured that the connection setter calls close on the previous connection. (Hopefully) Closes #18598 ## Validation Steps Performed * Can't repro the original failure ❌ * Opening and closing tabs as fast as possible doesn't crash anymore ✅ * Detaching and reattaching a tab producing continuous output ✅
This commit is contained in:
Leonard Hecker
GitHub
parent
7d8f7eb429
commit
70f85a4a35
@ -60,9 +60,12 @@ namespace winrt::Microsoft::TerminalApp::implementation
|
||||
|
||||
DebugTapConnection::DebugTapConnection(ITerminalConnection wrappedConnection)
|
||||
{
|
||||
_outputRevoker = wrappedConnection.TerminalOutput(winrt::auto_revoke, { this, &DebugTapConnection::_OutputHandler });
|
||||
_stateChangedRevoker = wrappedConnection.StateChanged(winrt::auto_revoke, [this](auto&& /*s*/, auto&& /*e*/) {
|
||||
StateChanged.raise(*this, nullptr);
|
||||
_outputRevoker = wrappedConnection.TerminalOutput(winrt::auto_revoke, { get_weak(), &DebugTapConnection::_OutputHandler });
|
||||
_stateChangedRevoker = wrappedConnection.StateChanged(winrt::auto_revoke, [weak = get_weak()](auto&& /*s*/, auto&& /*e*/) {
|
||||
if (const auto self = weak.get())
|
||||
{
|
||||
self->StateChanged.raise(*self, nullptr);
|
||||
}
|
||||
});
|
||||
_wrappedConnection = wrappedConnection;
|
||||
}
|
||||
|
||||
@ -142,23 +142,13 @@ namespace winrt::Microsoft::Terminal::Control::implementation
|
||||
// If we wait, a screen reader may try to get the AutomationPeer (aka the UIA Engine), and we won't be able to attach
|
||||
// the UIA Engine to the renderer. This prevents us from signaling changes to the cursor or buffer.
|
||||
{
|
||||
// First create the render thread.
|
||||
// Then stash a local pointer to the render thread so we can initialize it and enable it
|
||||
// to paint itself *after* we hand off its ownership to the renderer.
|
||||
// We split up construction and initialization of the render thread object this way
|
||||
// because the renderer and render thread have circular references to each other.
|
||||
auto renderThread = std::make_unique<::Microsoft::Console::Render::RenderThread>();
|
||||
auto* const localPointerToThread = renderThread.get();
|
||||
|
||||
// Now create the renderer and initialize the render thread.
|
||||
const auto& renderSettings = _terminal->GetRenderSettings();
|
||||
_renderer = std::make_unique<::Microsoft::Console::Render::Renderer>(renderSettings, _terminal.get(), nullptr, 0, std::move(renderThread));
|
||||
_renderer = std::make_unique<::Microsoft::Console::Render::Renderer>(renderSettings, _terminal.get());
|
||||
|
||||
_renderer->SetBackgroundColorChangedCallback([this]() { _rendererBackgroundColorChanged(); });
|
||||
_renderer->SetFrameColorChangedCallback([this]() { _rendererTabColorChanged(); });
|
||||
_renderer->SetRendererEnteredErrorStateCallback([this]() { RendererEnteredErrorState.raise(nullptr, nullptr); });
|
||||
|
||||
THROW_IF_FAILED(localPointerToThread->Initialize(_renderer.get()));
|
||||
}
|
||||
|
||||
UpdateSettings(settings, unfocusedAppearance);
|
||||
@ -186,7 +176,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
|
||||
// thread is a workaround for us to hit GH#12607 less often.
|
||||
shared->outputIdle = std::make_unique<til::debounced_func_trailing<>>(
|
||||
std::chrono::milliseconds{ 100 },
|
||||
[weakTerminal = std::weak_ptr{ _terminal }, weakThis = get_weak(), dispatcher = _dispatcher]() {
|
||||
[this, weakThis = get_weak(), dispatcher = _dispatcher]() {
|
||||
dispatcher.TryEnqueue(DispatcherQueuePriority::Normal, [weakThis]() {
|
||||
if (const auto self = weakThis.get(); self && !self->_IsClosing())
|
||||
{
|
||||
@ -194,22 +184,23 @@ namespace winrt::Microsoft::Terminal::Control::implementation
|
||||
}
|
||||
});
|
||||
|
||||
if (const auto t = weakTerminal.lock())
|
||||
{
|
||||
const auto lock = t->LockForWriting();
|
||||
t->UpdatePatternsUnderLock();
|
||||
}
|
||||
// We can't use a `weak_ptr` to `_terminal` here, because it takes significant
|
||||
// dependency on the lifetime of `this` (primarily on our `_renderer`).
|
||||
// and a `weak_ptr` would allow it to outlive `this`.
|
||||
// Theoretically `debounced_func_trailing` should call `WaitForThreadpoolTimerCallbacks()`
|
||||
// with cancel=true on destruction, which should ensure that our use of `this` here is safe.
|
||||
const auto lock = _terminal->LockForWriting();
|
||||
_terminal->UpdatePatternsUnderLock();
|
||||
});
|
||||
|
||||
// If you rapidly show/hide Windows Terminal, something about GotFocus()/LostFocus() gets broken.
|
||||
// We'll then receive easily 10+ such calls from WinUI the next time the application is shown.
|
||||
shared->focusChanged = std::make_unique<til::debounced_func_trailing<bool>>(
|
||||
std::chrono::milliseconds{ 25 },
|
||||
[weakThis = get_weak()](const bool focused) {
|
||||
if (const auto core{ weakThis.get() })
|
||||
{
|
||||
core->_focusChanged(focused);
|
||||
}
|
||||
[this](const bool focused) {
|
||||
// Theoretically `debounced_func_trailing` should call `WaitForThreadpoolTimerCallbacks()`
|
||||
// with cancel=true on destruction, which should ensure that our use of `this` here is safe.
|
||||
_focusChanged(focused);
|
||||
});
|
||||
|
||||
// Scrollbar updates are also expensive (XAML), so we'll throttle them as well.
|
||||
@ -224,19 +215,35 @@ namespace winrt::Microsoft::Terminal::Control::implementation
|
||||
});
|
||||
}
|
||||
|
||||
// Safely disconnects event handlers from the connection and closes it. This is necessary because
|
||||
// WinRT event revokers don't prevent pending calls from proceeding (thread-safe but not race-free).
|
||||
void ControlCore::_closeConnection()
|
||||
{
|
||||
_connectionOutputEventRevoker.revoke();
|
||||
_connectionStateChangedRevoker.revoke();
|
||||
|
||||
// One of the tasks for `ITerminalConnection::Close()` is to block until all pending
|
||||
// callback calls have completed. This solves the race-condition issue mentioned above.
|
||||
if (_connection)
|
||||
{
|
||||
_connection.Close();
|
||||
_connection = nullptr;
|
||||
}
|
||||
}
|
||||
|
||||
ControlCore::~ControlCore()
|
||||
{
|
||||
Close();
|
||||
|
||||
_renderer.reset();
|
||||
_renderEngine.reset();
|
||||
// See notes about the _renderer member in the header file.
|
||||
_renderer->TriggerTeardown();
|
||||
}
|
||||
|
||||
void ControlCore::Detach()
|
||||
{
|
||||
// Disable the renderer, so that it doesn't try to start any new frames
|
||||
// for our engines while we're not attached to anything.
|
||||
_renderer->WaitForPaintCompletionAndDisable(INFINITE);
|
||||
_renderer->TriggerTeardown();
|
||||
|
||||
// Clear out any throttled funcs that we had wired up to run on this UI
|
||||
// thread. These will be recreated in _setupDispatcherAndCallbacks, when
|
||||
@ -276,8 +283,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
|
||||
auto oldState = ConnectionState(); // rely on ControlCore's automatic null handling
|
||||
// revoke ALL old handlers immediately
|
||||
|
||||
_connectionOutputEventRevoker.revoke();
|
||||
_connectionStateChangedRevoker.revoke();
|
||||
_closeConnection();
|
||||
|
||||
_connection = newConnection;
|
||||
if (_connection)
|
||||
@ -366,7 +372,11 @@ namespace winrt::Microsoft::Terminal::Control::implementation
|
||||
const auto vp = _renderEngine->GetViewportInCharacters(viewInPixels);
|
||||
const auto width = vp.Width();
|
||||
const auto height = vp.Height();
|
||||
_connection.Resize(height, width);
|
||||
|
||||
if (_connection)
|
||||
{
|
||||
_connection.Resize(height, width);
|
||||
}
|
||||
|
||||
if (_owningHwnd != 0)
|
||||
{
|
||||
@ -420,6 +430,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
|
||||
{
|
||||
if (_initializedTerminal.load(std::memory_order_relaxed))
|
||||
{
|
||||
// The lock must be held, because it calls into IRenderData which is shared state.
|
||||
const auto lock = _terminal->LockForWriting();
|
||||
_renderer->EnablePainting();
|
||||
}
|
||||
@ -434,7 +445,10 @@ namespace winrt::Microsoft::Terminal::Control::implementation
|
||||
// - <none>
|
||||
void ControlCore::_sendInputToConnection(std::wstring_view wstr)
|
||||
{
|
||||
_connection.WriteInput(winrt_wstring_to_array_view(wstr));
|
||||
if (_connection)
|
||||
{
|
||||
_connection.WriteInput(winrt_wstring_to_array_view(wstr));
|
||||
}
|
||||
}
|
||||
|
||||
// Method Description:
|
||||
@ -471,7 +485,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
|
||||
const wchar_t CtrlD = 0x4;
|
||||
const wchar_t Enter = '\r';
|
||||
|
||||
if (_connection.State() >= winrt::Microsoft::Terminal::TerminalConnection::ConnectionState::Closed)
|
||||
if (_connection && _connection.State() >= winrt::Microsoft::Terminal::TerminalConnection::ConnectionState::Closed)
|
||||
{
|
||||
if (ch == CtrlD)
|
||||
{
|
||||
@ -1122,7 +1136,10 @@ namespace winrt::Microsoft::Terminal::Control::implementation
|
||||
return;
|
||||
}
|
||||
|
||||
_connection.Resize(vp.Height(), vp.Width());
|
||||
if (_connection)
|
||||
{
|
||||
_connection.Resize(vp.Height(), vp.Width());
|
||||
}
|
||||
|
||||
// TermControl will call Search() once the OutputIdle even fires after 100ms.
|
||||
// Until then we need to hide the now-stale search results from the renderer.
|
||||
@ -1794,12 +1811,9 @@ namespace winrt::Microsoft::Terminal::Control::implementation
|
||||
|
||||
// Ensure Close() doesn't hang, waiting for MidiAudio to finish playing an hour long song.
|
||||
_midiAudio.BeginSkip();
|
||||
|
||||
// Stop accepting new output and state changes before we disconnect everything.
|
||||
_connectionOutputEventRevoker.revoke();
|
||||
_connectionStateChangedRevoker.revoke();
|
||||
_connection.Close();
|
||||
}
|
||||
|
||||
_closeConnection();
|
||||
}
|
||||
|
||||
void ControlCore::PersistToPath(const wchar_t* path) const
|
||||
@ -1896,7 +1910,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
|
||||
|
||||
const auto weakThis{ get_weak() };
|
||||
|
||||
// Concurrent read of _dispatcher is safe, because Detach() calls WaitForPaintCompletionAndDisable()
|
||||
// Concurrent read of _dispatcher is safe, because Detach() calls TriggerTeardown()
|
||||
// which blocks until this call returns. _dispatcher will only be changed afterwards.
|
||||
co_await wil::resume_foreground(_dispatcher);
|
||||
|
||||
@ -1947,8 +1961,9 @@ namespace winrt::Microsoft::Terminal::Control::implementation
|
||||
|
||||
void ControlCore::ResumeRendering()
|
||||
{
|
||||
// The lock must be held, because it calls into IRenderData which is shared state.
|
||||
const auto lock = _terminal->LockForWriting();
|
||||
_renderer->ResetErrorStateAndResume();
|
||||
_renderer->EnablePainting();
|
||||
}
|
||||
|
||||
bool ControlCore::IsVtMouseModeEnabled() const
|
||||
|
||||
@ -311,65 +311,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation
|
||||
std::shared_ptr<ThrottledFuncTrailing<Control::ScrollPositionChangedArgs>> updateScrollBar;
|
||||
};
|
||||
|
||||
std::atomic<bool> _initializedTerminal{ false };
|
||||
bool _closing{ false };
|
||||
|
||||
TerminalConnection::ITerminalConnection _connection{ nullptr };
|
||||
TerminalConnection::ITerminalConnection::TerminalOutput_revoker _connectionOutputEventRevoker;
|
||||
TerminalConnection::ITerminalConnection::StateChanged_revoker _connectionStateChangedRevoker;
|
||||
|
||||
winrt::com_ptr<ControlSettings> _settings{ nullptr };
|
||||
|
||||
std::shared_ptr<::Microsoft::Terminal::Core::Terminal> _terminal{ nullptr };
|
||||
std::wstring _pendingResponses;
|
||||
|
||||
// NOTE: _renderEngine must be ordered before _renderer.
|
||||
//
|
||||
// As _renderer has a dependency on _renderEngine (through a raw pointer)
|
||||
// we must ensure the _renderer is deallocated first.
|
||||
// (C++ class members are destroyed in reverse order.)
|
||||
std::unique_ptr<::Microsoft::Console::Render::Atlas::AtlasEngine> _renderEngine{ nullptr };
|
||||
std::unique_ptr<::Microsoft::Console::Render::Renderer> _renderer{ nullptr };
|
||||
|
||||
::Search _searcher;
|
||||
bool _snapSearchResultToSelection;
|
||||
|
||||
winrt::handle _lastSwapChainHandle{ nullptr };
|
||||
|
||||
FontInfoDesired _desiredFont;
|
||||
FontInfo _actualFont;
|
||||
bool _builtinGlyphs = true;
|
||||
bool _colorGlyphs = true;
|
||||
CSSLengthPercentage _cellWidth;
|
||||
CSSLengthPercentage _cellHeight;
|
||||
|
||||
// storage location for the leading surrogate of a utf-16 surrogate pair
|
||||
std::optional<wchar_t> _leadingSurrogate{ std::nullopt };
|
||||
|
||||
std::optional<til::point> _lastHoveredCell{ std::nullopt };
|
||||
// Track the last hyperlink ID we hovered over
|
||||
uint16_t _lastHoveredId{ 0 };
|
||||
|
||||
bool _isReadOnly{ false };
|
||||
|
||||
std::optional<interval_tree::IntervalTree<til::point, size_t>::interval> _lastHoveredInterval{ std::nullopt };
|
||||
|
||||
// These members represent the size of the surface that we should be
|
||||
// rendering to.
|
||||
float _panelWidth{ 0 };
|
||||
float _panelHeight{ 0 };
|
||||
float _compositionScale{ 0 };
|
||||
|
||||
uint64_t _owningHwnd{ 0 };
|
||||
|
||||
winrt::Windows::System::DispatcherQueue _dispatcher{ nullptr };
|
||||
til::shared_mutex<SharedState> _shared;
|
||||
|
||||
til::point _contextMenuBufferPosition{ 0, 0 };
|
||||
|
||||
Windows::Foundation::Collections::IVector<hstring> _cachedQuickFixes{ nullptr };
|
||||
|
||||
void _setupDispatcherAndCallbacks();
|
||||
void _closeConnection();
|
||||
|
||||
bool _setFontSizeUnderLock(float fontSize);
|
||||
void _updateFont();
|
||||
@ -396,12 +339,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation
|
||||
void _terminalWindowSizeChanged(int32_t width, int32_t height);
|
||||
|
||||
safe_void_coroutine _terminalCompletionsChanged(std::wstring_view menuJson, unsigned int replaceLength);
|
||||
|
||||
#pragma endregion
|
||||
|
||||
MidiAudio _midiAudio;
|
||||
winrt::Windows::System::DispatcherQueueTimer _midiAudioSkipTimer{ nullptr };
|
||||
|
||||
#pragma region RendererCallbacks
|
||||
void _rendererWarning(const HRESULT hr, wil::zwstring_view parameter);
|
||||
safe_void_coroutine _renderEngineSwapChainChanged(const HANDLE handle);
|
||||
@ -412,6 +351,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
|
||||
void _raiseReadOnlyWarning();
|
||||
void _updateAntiAliasingMode();
|
||||
void _connectionOutputHandler(const hstring& hstr);
|
||||
void _connectionStateChangedHandler(const TerminalConnection::ITerminalConnection&, const Windows::Foundation::IInspectable&);
|
||||
void _updateHoveredCell(const std::optional<til::point> terminalPosition);
|
||||
void _setOpacity(const float opacity, const bool focused = true);
|
||||
|
||||
@ -443,6 +383,70 @@ namespace winrt::Microsoft::Terminal::Control::implementation
|
||||
return _closing;
|
||||
}
|
||||
|
||||
// Caches responses generated by our VT parser (= improved batching).
|
||||
std::wstring _pendingResponses;
|
||||
|
||||
// Font stuff.
|
||||
FontInfoDesired _desiredFont;
|
||||
FontInfo _actualFont;
|
||||
bool _builtinGlyphs = true;
|
||||
bool _colorGlyphs = true;
|
||||
CSSLengthPercentage _cellWidth;
|
||||
CSSLengthPercentage _cellHeight;
|
||||
|
||||
// Rendering stuff.
|
||||
winrt::handle _lastSwapChainHandle{ nullptr };
|
||||
uint64_t _owningHwnd{ 0 };
|
||||
float _panelWidth{ 0 };
|
||||
float _panelHeight{ 0 };
|
||||
float _compositionScale{ 0 };
|
||||
|
||||
// Audio stuff.
|
||||
MidiAudio _midiAudio;
|
||||
winrt::Windows::System::DispatcherQueueTimer _midiAudioSkipTimer{ nullptr };
|
||||
|
||||
// Other stuff.
|
||||
winrt::Windows::System::DispatcherQueue _dispatcher{ nullptr };
|
||||
winrt::com_ptr<ControlSettings> _settings{ nullptr };
|
||||
til::point _contextMenuBufferPosition{ 0, 0 };
|
||||
Windows::Foundation::Collections::IVector<hstring> _cachedQuickFixes{ nullptr };
|
||||
::Search _searcher;
|
||||
std::optional<interval_tree::IntervalTree<til::point, size_t>::interval> _lastHoveredInterval;
|
||||
std::optional<wchar_t> _leadingSurrogate;
|
||||
std::optional<til::point> _lastHoveredCell;
|
||||
uint16_t _lastHoveredId{ 0 };
|
||||
std::atomic<bool> _initializedTerminal{ false };
|
||||
bool _isReadOnly{ false };
|
||||
bool _closing{ false };
|
||||
|
||||
// ----------------------------------------------------------------------------------------
|
||||
// These are ordered last to ensure they're destroyed first.
|
||||
// This ensures that their respective contents stops taking dependency on the above.
|
||||
// I recommend reading the following paragraphs in reverse order.
|
||||
// ----------------------------------------------------------------------------------------
|
||||
|
||||
// ↑ This one is tricky - all of these are raw pointers:
|
||||
// 1. _terminal depends on _renderer (for invalidations)
|
||||
// 2. _renderer depends on _terminal (for IRenderData)
|
||||
// = circular dependency = architectural flaw (lifetime issues) = TODO
|
||||
// 3. _renderer depends on _renderEngine (AtlasEngine)
|
||||
// To solve the knot, we manually stop the renderer in the destructor,
|
||||
// which breaks 2. We can proceed then proceed to break 1. and then 3.
|
||||
std::unique_ptr<::Microsoft::Console::Render::Atlas::AtlasEngine> _renderEngine{ nullptr }; // 3.
|
||||
std::unique_ptr<::Microsoft::Console::Render::Renderer> _renderer{ nullptr }; // 3.
|
||||
std::shared_ptr<::Microsoft::Terminal::Core::Terminal> _terminal{ nullptr }; // 1.
|
||||
|
||||
// ↑ MOST IMPORTANTLY: `_outputIdle` takes dependency on the raw `this` pointer (necessarily).
|
||||
// Destroying SharedState here will block until all pending `debounced_func_trailing` calls are completed.
|
||||
til::shared_mutex<SharedState> _shared;
|
||||
|
||||
// ↑ Prevent any more unnecessary `_outputIdle` calls.
|
||||
// Technically none of these members are destroyed here. Instead, the destructor will call Close()
|
||||
// which calls _closeConnection() which in turn manually & safely destroys them in the correct order.
|
||||
TerminalConnection::ITerminalConnection::TerminalOutput_revoker _connectionOutputEventRevoker;
|
||||
TerminalConnection::ITerminalConnection::StateChanged_revoker _connectionStateChangedRevoker;
|
||||
TerminalConnection::ITerminalConnection _connection{ nullptr };
|
||||
|
||||
friend class ControlUnitTests::ControlCoreTests;
|
||||
friend class ControlUnitTests::ControlInteractivityTests;
|
||||
bool _inUnitTests{ false };
|
||||
|
||||
@ -73,7 +73,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
|
||||
//
|
||||
// To alleviate, make sure to disable the UIA engine and remove it,
|
||||
// and ALSO disable the renderer. Core.Detach will take care of the
|
||||
// WaitForPaintCompletionAndDisable (which will stop the renderer
|
||||
// TriggerTeardown (which will stop the renderer
|
||||
// after all current engines are done painting).
|
||||
//
|
||||
// Simply disabling the UIA engine is not enough, because it's
|
||||
|
||||
@ -206,14 +206,10 @@ 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();
|
||||
renderSettings.SetColorTableEntry(TextColor::DEFAULT_BACKGROUND, RGB(12, 12, 12));
|
||||
renderSettings.SetColorTableEntry(TextColor::DEFAULT_FOREGROUND, RGB(204, 204, 204));
|
||||
_renderer = std::make_unique<::Microsoft::Console::Render::Renderer>(renderSettings, _terminal.get(), nullptr, 0, std::move(renderThread));
|
||||
RETURN_HR_IF_NULL(E_POINTER, localPointerToThread);
|
||||
RETURN_IF_FAILED(localPointerToThread->Initialize(_renderer.get()));
|
||||
_renderer = std::make_unique<::Microsoft::Console::Render::Renderer>(renderSettings, _terminal.get());
|
||||
|
||||
auto engine = std::make_unique<::Microsoft::Console::Render::AtlasEngine>();
|
||||
RETURN_IF_FAILED(engine->SetHwnd(_hwnd.get()));
|
||||
@ -234,7 +230,7 @@ HRESULT HwndTerminal::Initialize()
|
||||
|
||||
_terminal->Create({ 80, 25 }, 9001, *_renderer);
|
||||
_terminal->SetWriteInputCallback([=](std::wstring_view input) noexcept { _WriteTextToConnection(input); });
|
||||
localPointerToThread->EnablePainting();
|
||||
_renderer->EnablePainting();
|
||||
|
||||
_multiClickTime = std::chrono::milliseconds{ GetDoubleClickTime() };
|
||||
|
||||
|
||||
@ -842,16 +842,7 @@ PWSTR TranslateConsoleTitle(_In_ PCWSTR pwszConsoleTitle, const BOOL fUnexpand,
|
||||
{
|
||||
if (!gci.IsInVtIoMode())
|
||||
{
|
||||
auto renderThread = std::make_unique<RenderThread>();
|
||||
// stash a local pointer to the thread here -
|
||||
// We're going to give ownership of the thread to the Renderer,
|
||||
// but the thread also need to be told who its renderer is,
|
||||
// and we can't do that until the renderer is constructed.
|
||||
auto* const localPointerToThread = renderThread.get();
|
||||
|
||||
g.pRender = new Renderer(gci.GetRenderSettings(), &gci.renderData, nullptr, 0, std::move(renderThread));
|
||||
|
||||
THROW_IF_FAILED(localPointerToThread->Initialize(g.pRender));
|
||||
g.pRender = new Renderer(gci.GetRenderSettings(), &gci.renderData);
|
||||
|
||||
// Set up the renderer to be used to calculate the width of a glyph,
|
||||
// should we be unable to figure out its width another way.
|
||||
|
||||
@ -44,7 +44,6 @@ class ScreenBufferTests
|
||||
|
||||
m_state->InitEvents();
|
||||
m_state->PrepareGlobalFont({ 1, 1 });
|
||||
m_state->PrepareGlobalRenderer();
|
||||
m_state->PrepareGlobalInputBuffer();
|
||||
m_state->PrepareGlobalScreenBuffer();
|
||||
|
||||
@ -54,7 +53,6 @@ class ScreenBufferTests
|
||||
TEST_CLASS_CLEANUP(ClassCleanup)
|
||||
{
|
||||
m_state->CleanupGlobalScreenBuffer();
|
||||
m_state->CleanupGlobalRenderer();
|
||||
m_state->CleanupGlobalInputBuffer();
|
||||
|
||||
delete m_state;
|
||||
@ -581,8 +579,6 @@ void ScreenBufferTests::TestResetClearTabStops()
|
||||
// Reset the screen buffer to test the defaults.
|
||||
m_state->CleanupNewTextBufferInfo();
|
||||
m_state->CleanupGlobalScreenBuffer();
|
||||
m_state->CleanupGlobalRenderer();
|
||||
m_state->PrepareGlobalRenderer();
|
||||
m_state->PrepareGlobalScreenBuffer();
|
||||
m_state->PrepareNewTextBufferInfo();
|
||||
|
||||
|
||||
@ -23,20 +23,14 @@ class SearchTests
|
||||
TEST_CLASS_SETUP(ClassSetup)
|
||||
{
|
||||
m_state = new CommonState();
|
||||
|
||||
m_state->PrepareGlobalRenderer();
|
||||
m_state->PrepareGlobalScreenBuffer();
|
||||
|
||||
return true;
|
||||
}
|
||||
|
||||
TEST_CLASS_CLEANUP(ClassCleanup)
|
||||
{
|
||||
m_state->CleanupGlobalScreenBuffer();
|
||||
m_state->CleanupGlobalRenderer();
|
||||
|
||||
delete m_state;
|
||||
|
||||
return true;
|
||||
}
|
||||
|
||||
|
||||
@ -66,20 +66,6 @@ public:
|
||||
m_pFontInfo = { L"Consolas", 0, 0, coordFontSize, 0 };
|
||||
}
|
||||
|
||||
void PrepareGlobalRenderer()
|
||||
{
|
||||
Globals& g = Microsoft::Console::Interactivity::ServiceLocator::LocateGlobals();
|
||||
CONSOLE_INFORMATION& gci = g.getConsoleInformation();
|
||||
g.pRender = new Microsoft::Console::Render::Renderer(gci.GetRenderSettings(), &gci.renderData, nullptr, 0, nullptr);
|
||||
}
|
||||
|
||||
void CleanupGlobalRenderer()
|
||||
{
|
||||
Globals& g = Microsoft::Console::Interactivity::ServiceLocator::LocateGlobals();
|
||||
delete g.pRender;
|
||||
g.pRender = nullptr;
|
||||
}
|
||||
|
||||
void PrepareGlobalScreenBuffer(const til::CoordType viewWidth = s_csWindowWidth,
|
||||
const til::CoordType viewHeight = s_csWindowHeight,
|
||||
const til::CoordType bufferWidth = s_csBufferWidth,
|
||||
|
||||
@ -393,7 +393,58 @@ VOID ConIoSrvComm::HandleFocusEvent(const CIS_EVENT* const Event)
|
||||
{
|
||||
// Wait for the currently running paint operation, if any,
|
||||
// and prevent further attempts to render.
|
||||
Renderer->WaitForPaintCompletionAndDisable(1000);
|
||||
//
|
||||
// When rendering takes place via DirectX, and a console application
|
||||
// currently owns the screen, and a new console application is launched (or
|
||||
// the user switches to another console application), the new application
|
||||
// cannot take over the screen until the active one relinquishes it. This
|
||||
// blocking mechanism goes as follows:
|
||||
//
|
||||
// 1. The console input thread of the new console application connects to
|
||||
// ConIoSrv;
|
||||
// 2. While servicing the new connection request, ConIoSrv sends an event to
|
||||
// the active application letting it know that it has lost focus;
|
||||
// 3.1 ConIoSrv waits for a reply from the client application;
|
||||
// 3.2 Meanwhile, the active application receives the focus event and calls
|
||||
// this method, waiting for the current paint operation to
|
||||
// finish.
|
||||
//
|
||||
// This means that the new application is waiting on the connection request
|
||||
// reply from ConIoSrv, ConIoSrv is waiting on the active application to
|
||||
// acknowledge the lost focus event to reply to the new application, and the
|
||||
// console input thread in the active application is waiting on the renderer
|
||||
// thread to finish its current paint operation.
|
||||
//
|
||||
// Question: what should happen if the wait on the paint operation times
|
||||
// out?
|
||||
//
|
||||
// There are three options:
|
||||
//
|
||||
// 1. On timeout, the active console application could reply with an error
|
||||
// message and terminate itself, effectively relinquishing control of the
|
||||
// display;
|
||||
//
|
||||
// 2. ConIoSrv itself could time out on waiting for a reply, and forcibly
|
||||
// terminate the active console application;
|
||||
//
|
||||
// 3. Let the wait time out and let the user deal with it. Because the wait
|
||||
// occurs on a single iteration of the renderer thread, it seemed to me that
|
||||
// the likelihood of failure is extremely small, especially since the client
|
||||
// console application that the active conhost instance is servicing has no
|
||||
// say over what happens in the renderer thread, only by proxy. Thus, the
|
||||
// chance of failure (timeout) is minimal and since the OneCoreUAP console
|
||||
// is not a massively used piece of software, it didn’t seem that it would
|
||||
// be a good use of time to build the requisite infrastructure to deal with
|
||||
// a timeout here, at least not for now. In case of a timeout DirectX will
|
||||
// catch the mistake of a new application attempting to acquire the display
|
||||
// while another one still owns it and will flag it as a DWM bug. Right now,
|
||||
// the active application will wait one second for the paint operation to
|
||||
// finish.
|
||||
//
|
||||
// TODO: MSFT: 11833883 - Determine action when wait on paint operation via
|
||||
// DirectX on OneCoreUAP times out while switching console
|
||||
// applications.
|
||||
Renderer->TriggerTeardown();
|
||||
|
||||
// Relinquish control of the graphics device (only one
|
||||
// DirectX application may control the device at any one
|
||||
|
||||
@ -25,35 +25,12 @@ static constexpr auto renderBackoffBaseTimeMilliseconds{ 150 };
|
||||
// - Creates a new renderer controller for a console.
|
||||
// Arguments:
|
||||
// - pData - The interface to console data structures required for rendering
|
||||
// - pEngine - The output engine for targeting each rendering frame
|
||||
// Return Value:
|
||||
// - An instance of a Renderer.
|
||||
Renderer::Renderer(const RenderSettings& renderSettings,
|
||||
IRenderData* pData,
|
||||
_In_reads_(cEngines) IRenderEngine** const rgpEngines,
|
||||
const size_t cEngines,
|
||||
std::unique_ptr<RenderThread> thread) :
|
||||
Renderer::Renderer(const RenderSettings& renderSettings, IRenderData* pData) :
|
||||
_renderSettings(renderSettings),
|
||||
_pData(pData),
|
||||
_pThread{ std::move(thread) }
|
||||
_pData(pData)
|
||||
{
|
||||
for (size_t i = 0; i < cEngines; i++)
|
||||
{
|
||||
AddRenderEngine(rgpEngines[i]);
|
||||
}
|
||||
}
|
||||
|
||||
// Routine Description:
|
||||
// - Destroys an instance of a renderer
|
||||
// Arguments:
|
||||
// - <none>
|
||||
// Return Value:
|
||||
// - <none>
|
||||
Renderer::~Renderer()
|
||||
{
|
||||
// RenderThread blocks until it has shut down.
|
||||
_destructing = true;
|
||||
_pThread.reset();
|
||||
}
|
||||
|
||||
IRenderData* Renderer::GetRenderData() const noexcept
|
||||
@ -72,11 +49,6 @@ IRenderData* Renderer::GetRenderData() const noexcept
|
||||
auto tries = maxRetriesForRenderEngine;
|
||||
while (tries > 0)
|
||||
{
|
||||
if (_destructing)
|
||||
{
|
||||
return S_FALSE;
|
||||
}
|
||||
|
||||
// BODGY: Optimally we would want to retry per engine, but that causes different
|
||||
// problems (intermittent inconsistent states between text renderer and UIA output,
|
||||
// not being able to lock the cursor location, etc.).
|
||||
@ -91,7 +63,7 @@ IRenderData* Renderer::GetRenderData() const noexcept
|
||||
if (--tries == 0)
|
||||
{
|
||||
// Stop trying.
|
||||
_pThread->DisablePainting();
|
||||
_thread.DisablePainting();
|
||||
if (_pfnRendererEnteredErrorState)
|
||||
{
|
||||
_pfnRendererEnteredErrorState();
|
||||
@ -207,12 +179,8 @@ CATCH_RETURN()
|
||||
|
||||
void Renderer::NotifyPaintFrame() noexcept
|
||||
{
|
||||
// If we're running in the unittests, we might not have a render thread.
|
||||
if (_pThread)
|
||||
{
|
||||
// The thread will provide throttling for us.
|
||||
_pThread->NotifyPaint();
|
||||
}
|
||||
// The thread will provide throttling for us.
|
||||
_thread.NotifyPaint();
|
||||
}
|
||||
|
||||
// Routine Description:
|
||||
@ -315,7 +283,7 @@ void Renderer::TriggerRedrawAll(const bool backgroundChanged, const bool frameCh
|
||||
void Renderer::TriggerTeardown() noexcept
|
||||
{
|
||||
// We need to shut down the paint thread on teardown.
|
||||
_pThread->WaitForPaintCompletionAndDisable(INFINITE);
|
||||
_thread.TriggerTeardown();
|
||||
}
|
||||
|
||||
// Routine Description:
|
||||
@ -637,25 +605,7 @@ void Renderer::EnablePainting()
|
||||
// When the renderer is constructed, the initial viewport won't be available yet,
|
||||
// but once EnablePainting is called it should be safe to retrieve.
|
||||
_viewport = _pData->GetViewport();
|
||||
|
||||
// When running the unit tests, we may be using a render without a render thread.
|
||||
if (_pThread)
|
||||
{
|
||||
_pThread->EnablePainting();
|
||||
}
|
||||
}
|
||||
|
||||
// Routine Description:
|
||||
// - Waits for the current paint operation to complete, if any, up to the specified timeout.
|
||||
// - Resets an event in the render thread that precludes it from advancing, thus disabling rendering.
|
||||
// - If no paint operation is currently underway, returns immediately.
|
||||
// Arguments:
|
||||
// - dwTimeoutMs - Milliseconds to wait for the current paint operation to complete, if any (can be INFINITE).
|
||||
// Return Value:
|
||||
// - <none>
|
||||
void Renderer::WaitForPaintCompletionAndDisable(const DWORD dwTimeoutMs)
|
||||
{
|
||||
_pThread->WaitForPaintCompletionAndDisable(dwTimeoutMs);
|
||||
_thread.EnablePainting();
|
||||
}
|
||||
|
||||
// Routine Description:
|
||||
@ -1437,14 +1387,6 @@ void Renderer::SetRendererEnteredErrorStateCallback(std::function<void()> pfn)
|
||||
_pfnRendererEnteredErrorState = std::move(pfn);
|
||||
}
|
||||
|
||||
// Method Description:
|
||||
// - Attempts to restart the renderer.
|
||||
void Renderer::ResetErrorStateAndResume()
|
||||
{
|
||||
// because we're not stateful (we could be in the future), all we want to do is reenable painting.
|
||||
EnablePainting();
|
||||
}
|
||||
|
||||
void Renderer::UpdateHyperlinkHoveredId(uint16_t id) noexcept
|
||||
{
|
||||
_hyperlinkHoveredId = id;
|
||||
|
||||
@ -28,13 +28,7 @@ namespace Microsoft::Console::Render
|
||||
class Renderer
|
||||
{
|
||||
public:
|
||||
Renderer(const RenderSettings& renderSettings,
|
||||
IRenderData* pData,
|
||||
_In_reads_(cEngines) IRenderEngine** const pEngine,
|
||||
const size_t cEngines,
|
||||
std::unique_ptr<RenderThread> thread);
|
||||
|
||||
~Renderer();
|
||||
Renderer(const RenderSettings& renderSettings, IRenderData* pData);
|
||||
|
||||
IRenderData* GetRenderData() const noexcept;
|
||||
|
||||
@ -71,7 +65,6 @@ namespace Microsoft::Console::Render
|
||||
bool IsGlyphWideByFont(const std::wstring_view glyph);
|
||||
|
||||
void EnablePainting();
|
||||
void WaitForPaintCompletionAndDisable(const DWORD dwTimeoutMs);
|
||||
void WaitUntilCanRender();
|
||||
|
||||
void AddRenderEngine(_In_ IRenderEngine* const pEngine);
|
||||
@ -80,7 +73,6 @@ namespace Microsoft::Console::Render
|
||||
void SetBackgroundColorChangedCallback(std::function<void()> pfn);
|
||||
void SetFrameColorChangedCallback(std::function<void()> pfn);
|
||||
void SetRendererEnteredErrorStateCallback(std::function<void()> pfn);
|
||||
void ResetErrorStateAndResume();
|
||||
|
||||
void UpdateHyperlinkHoveredId(uint16_t id) noexcept;
|
||||
void UpdateLastHoveredInterval(const std::optional<interval_tree::IntervalTree<til::point, size_t>::interval>& newInterval);
|
||||
@ -121,23 +113,25 @@ namespace Microsoft::Console::Render
|
||||
const RenderSettings& _renderSettings;
|
||||
std::array<IRenderEngine*, 2> _engines{};
|
||||
IRenderData* _pData = nullptr; // Non-ownership pointer
|
||||
std::unique_ptr<RenderThread> _pThread;
|
||||
static constexpr size_t _firstSoftFontChar = 0xEF20;
|
||||
size_t _lastSoftFontChar = 0;
|
||||
uint16_t _hyperlinkHoveredId = 0;
|
||||
std::optional<interval_tree::IntervalTree<til::point, size_t>::interval> _hoveredInterval;
|
||||
Microsoft::Console::Types::Viewport _viewport;
|
||||
CursorOptions _currentCursorOptions;
|
||||
CursorOptions _currentCursorOptions{};
|
||||
std::optional<CompositionCache> _compositionCache;
|
||||
std::vector<Cluster> _clusterBuffer;
|
||||
std::function<void()> _pfnBackgroundColorChanged;
|
||||
std::function<void()> _pfnFrameColorChanged;
|
||||
std::function<void()> _pfnRendererEnteredErrorState;
|
||||
bool _destructing = false;
|
||||
bool _forceUpdateViewport = false;
|
||||
|
||||
til::point_span _lastSelectionPaintSpan{};
|
||||
size_t _lastSelectionPaintSize{};
|
||||
std::vector<til::rect> _lastSelectionRectsByViewport{};
|
||||
|
||||
// Ordered last, so that it gets destroyed first.
|
||||
// This ensures that the render thread stops accessing us.
|
||||
RenderThread _thread{ this };
|
||||
};
|
||||
}
|
||||
|
||||
@ -10,212 +10,40 @@
|
||||
|
||||
using namespace Microsoft::Console::Render;
|
||||
|
||||
RenderThread::RenderThread() :
|
||||
_pRenderer(nullptr),
|
||||
_hThread(nullptr),
|
||||
_hEvent(nullptr),
|
||||
_hPaintCompletedEvent(nullptr),
|
||||
_fKeepRunning(true),
|
||||
_hPaintEnabledEvent(nullptr),
|
||||
_fNextFrameRequested(false),
|
||||
_fWaiting(false)
|
||||
RenderThread::RenderThread(Renderer* renderer) :
|
||||
renderer(renderer)
|
||||
{
|
||||
}
|
||||
|
||||
RenderThread::~RenderThread()
|
||||
{
|
||||
if (_hThread)
|
||||
{
|
||||
_fKeepRunning = false; // stop loop after final run
|
||||
EnablePainting(); // if we want to get the last frame out, we need to make sure it's enabled
|
||||
SignalObjectAndWait(_hEvent, _hThread, INFINITE, FALSE); // signal final paint and wait for thread to finish.
|
||||
|
||||
CloseHandle(_hThread);
|
||||
_hThread = nullptr;
|
||||
}
|
||||
|
||||
if (_hEvent)
|
||||
{
|
||||
CloseHandle(_hEvent);
|
||||
_hEvent = nullptr;
|
||||
}
|
||||
|
||||
if (_hPaintEnabledEvent)
|
||||
{
|
||||
CloseHandle(_hPaintEnabledEvent);
|
||||
_hPaintEnabledEvent = nullptr;
|
||||
}
|
||||
|
||||
if (_hPaintCompletedEvent)
|
||||
{
|
||||
CloseHandle(_hPaintCompletedEvent);
|
||||
_hPaintCompletedEvent = nullptr;
|
||||
}
|
||||
}
|
||||
|
||||
// Method Description:
|
||||
// - Create all of the Events we'll need, and the actual thread we'll be doing
|
||||
// work on.
|
||||
// Arguments:
|
||||
// - pRendererParent: the Renderer that owns this thread, and which we should
|
||||
// trigger frames for.
|
||||
// Return Value:
|
||||
// - S_OK if we succeeded, else an HRESULT corresponding to a failure to create
|
||||
// an Event or Thread.
|
||||
[[nodiscard]] HRESULT RenderThread::Initialize(Renderer* const pRendererParent) noexcept
|
||||
{
|
||||
_pRenderer = pRendererParent;
|
||||
|
||||
auto hr = S_OK;
|
||||
// Create event before thread as thread will start immediately.
|
||||
if (SUCCEEDED(hr))
|
||||
{
|
||||
auto hEvent = CreateEventW(nullptr, // non-inheritable security attributes
|
||||
FALSE, // auto reset event
|
||||
FALSE, // initially unsignaled
|
||||
nullptr // no name
|
||||
);
|
||||
|
||||
if (hEvent == nullptr)
|
||||
{
|
||||
hr = HRESULT_FROM_WIN32(GetLastError());
|
||||
}
|
||||
else
|
||||
{
|
||||
_hEvent = hEvent;
|
||||
}
|
||||
}
|
||||
|
||||
if (SUCCEEDED(hr))
|
||||
{
|
||||
auto hPaintEnabledEvent = CreateEventW(nullptr,
|
||||
TRUE, // manual reset event
|
||||
FALSE, // initially signaled
|
||||
nullptr);
|
||||
|
||||
if (hPaintEnabledEvent == nullptr)
|
||||
{
|
||||
hr = HRESULT_FROM_WIN32(GetLastError());
|
||||
}
|
||||
else
|
||||
{
|
||||
_hPaintEnabledEvent = hPaintEnabledEvent;
|
||||
}
|
||||
}
|
||||
|
||||
if (SUCCEEDED(hr))
|
||||
{
|
||||
auto hPaintCompletedEvent = CreateEventW(nullptr,
|
||||
TRUE, // manual reset event
|
||||
TRUE, // initially signaled
|
||||
nullptr);
|
||||
|
||||
if (hPaintCompletedEvent == nullptr)
|
||||
{
|
||||
hr = HRESULT_FROM_WIN32(GetLastError());
|
||||
}
|
||||
else
|
||||
{
|
||||
_hPaintCompletedEvent = hPaintCompletedEvent;
|
||||
}
|
||||
}
|
||||
|
||||
if (SUCCEEDED(hr))
|
||||
{
|
||||
auto hThread = CreateThread(nullptr, // non-inheritable security attributes
|
||||
0, // use default stack size
|
||||
s_ThreadProc,
|
||||
this,
|
||||
0, // create immediately
|
||||
nullptr // we don't need the thread ID
|
||||
);
|
||||
|
||||
if (hThread == nullptr)
|
||||
{
|
||||
hr = HRESULT_FROM_WIN32(GetLastError());
|
||||
}
|
||||
else
|
||||
{
|
||||
_hThread = hThread;
|
||||
|
||||
// SetThreadDescription only works on 1607 and higher. If we cannot find it,
|
||||
// then it's no big deal. Just skip setting the description.
|
||||
auto func = GetProcAddressByFunctionDeclaration(GetModuleHandleW(L"kernel32.dll"), SetThreadDescription);
|
||||
if (func)
|
||||
{
|
||||
LOG_IF_FAILED(func(hThread, L"Rendering Output Thread"));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return hr;
|
||||
TriggerTeardown();
|
||||
}
|
||||
|
||||
DWORD WINAPI RenderThread::s_ThreadProc(_In_ LPVOID lpParameter)
|
||||
{
|
||||
const auto pContext = static_cast<RenderThread*>(lpParameter);
|
||||
|
||||
if (pContext != nullptr)
|
||||
{
|
||||
return pContext->_ThreadProc();
|
||||
}
|
||||
else
|
||||
{
|
||||
return (DWORD)E_INVALIDARG;
|
||||
}
|
||||
return pContext->_ThreadProc();
|
||||
}
|
||||
|
||||
DWORD WINAPI RenderThread::_ThreadProc()
|
||||
{
|
||||
while (_fKeepRunning)
|
||||
while (true)
|
||||
{
|
||||
_enable.wait();
|
||||
|
||||
// Between waiting on _hEvent and calling PaintFrame() there should be a minimal delay,
|
||||
// so that a key press progresses to a drawing operation as quickly as possible.
|
||||
// As such, we wait for the renderer to complete _before_ waiting on _hEvent.
|
||||
_pRenderer->WaitUntilCanRender();
|
||||
renderer->WaitUntilCanRender();
|
||||
|
||||
WaitForSingleObject(_hPaintEnabledEvent, INFINITE);
|
||||
|
||||
if (!_fNextFrameRequested.exchange(false, std::memory_order_acq_rel))
|
||||
_redraw.wait();
|
||||
if (!_keepRunning.load(std::memory_order_relaxed))
|
||||
{
|
||||
// <--
|
||||
// If `NotifyPaint` is called at this point, then it will not
|
||||
// set the event because `_fWaiting` is not `true` yet so we have
|
||||
// to check again below.
|
||||
|
||||
_fWaiting.store(true, std::memory_order_release);
|
||||
|
||||
// check again now (see comment above)
|
||||
if (!_fNextFrameRequested.exchange(false, std::memory_order_acq_rel))
|
||||
{
|
||||
// Wait until a next frame is requested.
|
||||
WaitForSingleObject(_hEvent, INFINITE);
|
||||
}
|
||||
|
||||
// <--
|
||||
// If `NotifyPaint` is called at this point, then it _will_ set
|
||||
// the event because `_fWaiting` is `true`, but we're not waiting
|
||||
// anymore!
|
||||
// This can probably happen quite often: imagine a scenario where
|
||||
// we are waiting, and the terminal calls `NotifyPaint` twice
|
||||
// very quickly.
|
||||
// In that case, both calls might end up calling `SetEvent`. The
|
||||
// first one will resume this thread and the second one will
|
||||
// `SetEvent` the event. So the next time we wait, the event will
|
||||
// already be set and we won't actually wait.
|
||||
// Because it can happen often, and because rendering is an
|
||||
// expensive operation, we should reset the event to not render
|
||||
// again if nothing changed.
|
||||
|
||||
_fWaiting.store(false, std::memory_order_release);
|
||||
|
||||
// see comment above
|
||||
ResetEvent(_hEvent);
|
||||
break;
|
||||
}
|
||||
|
||||
ResetEvent(_hPaintCompletedEvent);
|
||||
LOG_IF_FAILED(_pRenderer->PaintFrame());
|
||||
SetEvent(_hPaintCompletedEvent);
|
||||
LOG_IF_FAILED(renderer->PaintFrame());
|
||||
}
|
||||
|
||||
return S_OK;
|
||||
@ -223,79 +51,54 @@ DWORD WINAPI RenderThread::_ThreadProc()
|
||||
|
||||
void RenderThread::NotifyPaint() noexcept
|
||||
{
|
||||
if (_fWaiting.load(std::memory_order_acquire))
|
||||
{
|
||||
SetEvent(_hEvent);
|
||||
}
|
||||
else
|
||||
{
|
||||
_fNextFrameRequested.store(true, std::memory_order_release);
|
||||
}
|
||||
_redraw.SetEvent();
|
||||
}
|
||||
|
||||
// Spawns a new rendering thread if none exists yet.
|
||||
void RenderThread::EnablePainting() noexcept
|
||||
{
|
||||
SetEvent(_hPaintEnabledEvent);
|
||||
const auto guard = _threadMutex.lock_exclusive();
|
||||
|
||||
_enable.SetEvent();
|
||||
|
||||
if (!_thread)
|
||||
{
|
||||
_keepRunning.store(true, std::memory_order_relaxed);
|
||||
|
||||
_thread.reset(CreateThread(nullptr, 0, s_ThreadProc, this, 0, nullptr));
|
||||
THROW_LAST_ERROR_IF(!_thread);
|
||||
|
||||
// SetThreadDescription only works on 1607 and higher. If we cannot find it,
|
||||
// then it's no big deal. Just skip setting the description.
|
||||
const auto func = GetProcAddressByFunctionDeclaration(GetModuleHandleW(L"kernel32.dll"), SetThreadDescription);
|
||||
if (func)
|
||||
{
|
||||
LOG_IF_FAILED(func(_thread.get(), L"Rendering Output Thread"));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
void RenderThread::DisablePainting() noexcept
|
||||
{
|
||||
ResetEvent(_hPaintEnabledEvent);
|
||||
_enable.ResetEvent();
|
||||
}
|
||||
|
||||
void RenderThread::WaitForPaintCompletionAndDisable(const DWORD dwTimeoutMs) noexcept
|
||||
// Stops the rendering thread, and waits for it to finish.
|
||||
void RenderThread::TriggerTeardown() noexcept
|
||||
{
|
||||
// When rendering takes place via DirectX, and a console application
|
||||
// currently owns the screen, and a new console application is launched (or
|
||||
// the user switches to another console application), the new application
|
||||
// cannot take over the screen until the active one relinquishes it. This
|
||||
// blocking mechanism goes as follows:
|
||||
//
|
||||
// 1. The console input thread of the new console application connects to
|
||||
// ConIoSrv;
|
||||
// 2. While servicing the new connection request, ConIoSrv sends an event to
|
||||
// the active application letting it know that it has lost focus;
|
||||
// 3.1 ConIoSrv waits for a reply from the client application;
|
||||
// 3.2 Meanwhile, the active application receives the focus event and calls
|
||||
// this method, waiting for the current paint operation to
|
||||
// finish.
|
||||
//
|
||||
// This means that the new application is waiting on the connection request
|
||||
// reply from ConIoSrv, ConIoSrv is waiting on the active application to
|
||||
// acknowledge the lost focus event to reply to the new application, and the
|
||||
// console input thread in the active application is waiting on the renderer
|
||||
// thread to finish its current paint operation.
|
||||
//
|
||||
// Question: what should happen if the wait on the paint operation times
|
||||
// out?
|
||||
//
|
||||
// There are three options:
|
||||
//
|
||||
// 1. On timeout, the active console application could reply with an error
|
||||
// message and terminate itself, effectively relinquishing control of the
|
||||
// display;
|
||||
//
|
||||
// 2. ConIoSrv itself could time out on waiting for a reply, and forcibly
|
||||
// terminate the active console application;
|
||||
//
|
||||
// 3. Let the wait time out and let the user deal with it. Because the wait
|
||||
// occurs on a single iteration of the renderer thread, it seemed to me that
|
||||
// the likelihood of failure is extremely small, especially since the client
|
||||
// console application that the active conhost instance is servicing has no
|
||||
// say over what happens in the renderer thread, only by proxy. Thus, the
|
||||
// chance of failure (timeout) is minimal and since the OneCoreUAP console
|
||||
// is not a massively used piece of software, it didn’t seem that it would
|
||||
// be a good use of time to build the requisite infrastructure to deal with
|
||||
// a timeout here, at least not for now. In case of a timeout DirectX will
|
||||
// catch the mistake of a new application attempting to acquire the display
|
||||
// while another one still owns it and will flag it as a DWM bug. Right now,
|
||||
// the active application will wait one second for the paint operation to
|
||||
// finish.
|
||||
//
|
||||
// TODO: MSFT: 11833883 - Determine action when wait on paint operation via
|
||||
// DirectX on OneCoreUAP times out while switching console
|
||||
// applications.
|
||||
const auto guard = _threadMutex.lock_exclusive();
|
||||
|
||||
ResetEvent(_hPaintEnabledEvent);
|
||||
WaitForSingleObject(_hPaintCompletedEvent, dwTimeoutMs);
|
||||
if (_thread)
|
||||
{
|
||||
// The render thread first waits for the event and then checks _keepRunning. By doing it
|
||||
// in reverse order here, we ensure that it's impossible for the render thread to miss this.
|
||||
_keepRunning.store(false, std::memory_order_relaxed);
|
||||
_redraw.SetEvent();
|
||||
_enable.SetEvent();
|
||||
|
||||
WaitForSingleObject(_thread.get(), INFINITE);
|
||||
_thread.reset();
|
||||
}
|
||||
|
||||
DisablePainting();
|
||||
}
|
||||
|
||||
@ -21,30 +21,23 @@ namespace Microsoft::Console::Render
|
||||
class RenderThread
|
||||
{
|
||||
public:
|
||||
RenderThread();
|
||||
RenderThread(Renderer* renderer);
|
||||
~RenderThread();
|
||||
|
||||
[[nodiscard]] HRESULT Initialize(Renderer* const pRendererParent) noexcept;
|
||||
|
||||
void NotifyPaint() noexcept;
|
||||
void EnablePainting() noexcept;
|
||||
void DisablePainting() noexcept;
|
||||
void WaitForPaintCompletionAndDisable(const DWORD dwTimeoutMs) noexcept;
|
||||
void TriggerTeardown() noexcept;
|
||||
|
||||
private:
|
||||
static DWORD WINAPI s_ThreadProc(_In_ LPVOID lpParameter);
|
||||
DWORD WINAPI _ThreadProc();
|
||||
|
||||
HANDLE _hThread;
|
||||
HANDLE _hEvent;
|
||||
|
||||
HANDLE _hPaintEnabledEvent;
|
||||
HANDLE _hPaintCompletedEvent;
|
||||
|
||||
Renderer* _pRenderer; // Non-ownership pointer
|
||||
|
||||
bool _fKeepRunning;
|
||||
std::atomic<bool> _fNextFrameRequested;
|
||||
std::atomic<bool> _fWaiting;
|
||||
Renderer* renderer;
|
||||
wil::slim_event_manual_reset _enable;
|
||||
wil::slim_event_auto_reset _redraw;
|
||||
wil::srwlock _threadMutex;
|
||||
wil::unique_handle _thread;
|
||||
std::atomic<bool> _keepRunning{ false };
|
||||
};
|
||||
}
|
||||
|
||||
@ -18,7 +18,7 @@ class DummyRenderer final : public Microsoft::Console::Render::Renderer
|
||||
{
|
||||
public:
|
||||
DummyRenderer(Microsoft::Console::Render::IRenderData* pData = nullptr) :
|
||||
Microsoft::Console::Render::Renderer(_renderSettings, pData, nullptr, 0, nullptr) {}
|
||||
Microsoft::Console::Render::Renderer(_renderSettings, pData) {}
|
||||
|
||||
Microsoft::Console::Render::RenderSettings _renderSettings;
|
||||
};
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user