diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 18ebdd96011..76b68a08d4f 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -158,7 +158,7 @@ Once you've discussed your proposed feature/fix/etc. with a team member, and you ### Testing -Testing is a key component in the development workflow. Both Windows Terminal and Windows Console use TAEF(the Test Authoring and Execution Framework) as the main framework for testing. +Testing is a key component in the development workflow. Both Windows Terminal and Windows Console use TAEF (the Test Authoring and Execution Framework) as the main framework for testing. If your changes affect existing test cases, or you're working on brand new features and also the accompanying test cases, see [TAEF](./doc/TAEF.md) for more information about how to validate your work locally. diff --git a/OpenConsole.sln b/OpenConsole.sln index a92c45a48ac..7698af450db 100644 --- a/OpenConsole.sln +++ b/OpenConsole.sln @@ -2304,10 +2304,14 @@ Global {6515F03F-E56D-4DB4-B23D-AC4FB80DB36F}.Release|x64.Build.0 = Release|x64 {6515F03F-E56D-4DB4-B23D-AC4FB80DB36F}.Release|x86.ActiveCfg = Release|Win32 {6515F03F-E56D-4DB4-B23D-AC4FB80DB36F}.Release|x86.Build.0 = Release|Win32 - {7615F03F-E56D-4DB4-B23D-BD4FB80DB36F}.AuditMode|Any CPU.ActiveCfg = Debug|Win32 - {7615F03F-E56D-4DB4-B23D-BD4FB80DB36F}.AuditMode|ARM64.ActiveCfg = Release|ARM64 - {7615F03F-E56D-4DB4-B23D-BD4FB80DB36F}.AuditMode|x64.ActiveCfg = Release|x64 - {7615F03F-E56D-4DB4-B23D-BD4FB80DB36F}.AuditMode|x86.ActiveCfg = Release|Win32 + {7615F03F-E56D-4DB4-B23D-BD4FB80DB36F}.AuditMode|Any CPU.ActiveCfg = AuditMode|x64 + {7615F03F-E56D-4DB4-B23D-BD4FB80DB36F}.AuditMode|Any CPU.Build.0 = AuditMode|x64 + {7615F03F-E56D-4DB4-B23D-BD4FB80DB36F}.AuditMode|ARM64.ActiveCfg = AuditMode|ARM64 + {7615F03F-E56D-4DB4-B23D-BD4FB80DB36F}.AuditMode|ARM64.Build.0 = AuditMode|ARM64 + {7615F03F-E56D-4DB4-B23D-BD4FB80DB36F}.AuditMode|x64.ActiveCfg = AuditMode|x64 + {7615F03F-E56D-4DB4-B23D-BD4FB80DB36F}.AuditMode|x64.Build.0 = AuditMode|x64 + {7615F03F-E56D-4DB4-B23D-BD4FB80DB36F}.AuditMode|x86.ActiveCfg = AuditMode|Win32 + {7615F03F-E56D-4DB4-B23D-BD4FB80DB36F}.AuditMode|x86.Build.0 = AuditMode|Win32 {7615F03F-E56D-4DB4-B23D-BD4FB80DB36F}.Debug|Any CPU.ActiveCfg = Debug|x64 {7615F03F-E56D-4DB4-B23D-BD4FB80DB36F}.Debug|ARM64.ActiveCfg = Debug|ARM64 {7615F03F-E56D-4DB4-B23D-BD4FB80DB36F}.Debug|ARM64.Build.0 = Debug|ARM64 diff --git a/src/cascadia/UIMarkdown/CodeBlock.h b/src/cascadia/UIMarkdown/CodeBlock.h index e80f45bf47a..81eaf63cb69 100644 --- a/src/cascadia/UIMarkdown/CodeBlock.h +++ b/src/cascadia/UIMarkdown/CodeBlock.h @@ -30,7 +30,7 @@ namespace winrt::Microsoft::Terminal::UI::Markdown::implementation struct RequestRunCommandsArgs : RequestRunCommandsArgsT { - RequestRunCommandsArgs(const winrt::hstring& commandlines) : + RequestRunCommandsArgs(const winrt::hstring& commandlines) noexcept : Commandlines{ commandlines } {}; til::property Commandlines; diff --git a/src/cascadia/UIMarkdown/MarkdownToXaml.cpp b/src/cascadia/UIMarkdown/MarkdownToXaml.cpp index 8933eb6a56f..81e70155f2f 100644 --- a/src/cascadia/UIMarkdown/MarkdownToXaml.cpp +++ b/src/cascadia/UIMarkdown/MarkdownToXaml.cpp @@ -92,7 +92,7 @@ WUX::Documents::Paragraph MarkdownToXaml::_CurrentParagraph() { _lastParagraph.TextIndent(-WidthOfBulletPoint); } - _lastParagraph.Margin(WUX::ThicknessHelper::FromLengths(IndentWidth * _indent, 0, 0, 0)); + _lastParagraph.Margin(WUX::ThicknessHelper::FromLengths(static_cast(IndentWidth) * _indent, 0, 0, 0)); } _root.Blocks().Append(_lastParagraph); } @@ -137,16 +137,16 @@ WUX::Documents::Run MarkdownToXaml::_NewRun() } return _lastRun; } -void MarkdownToXaml::_EndRun() +void MarkdownToXaml::_EndRun() noexcept { _lastRun = nullptr; } -void MarkdownToXaml::_EndSpan() +void MarkdownToXaml::_EndSpan() noexcept { _EndRun(); _lastSpan = nullptr; } -void MarkdownToXaml::_EndParagraph() +void MarkdownToXaml::_EndParagraph() noexcept { _EndSpan(); _lastParagraph = nullptr; @@ -162,7 +162,7 @@ WUX::Controls::TextBlock MarkdownToXaml::_makeDefaultTextBlock() void MarkdownToXaml::_RenderNode(cmark_node* node, cmark_event_type ev_type) { - bool entering = (ev_type == CMARK_EVENT_ENTER); + const bool entering = (ev_type == CMARK_EVENT_ENTER); switch (cmark_node_get_type(node)) { @@ -221,7 +221,7 @@ void MarkdownToXaml::_RenderNode(cmark_node* node, cmark_event_type ev_type) if (entering) { _EndParagraph(); - _NewRun().Text(bullets[std::clamp(_indent - _blockQuoteDepth - 1, 0, 2)]); + _NewRun().Text(gsl::at(bullets, std::clamp(_indent - _blockQuoteDepth - 1, 0, 2))); } break; @@ -251,7 +251,7 @@ void MarkdownToXaml::_RenderNode(cmark_node* node, cmark_event_type ev_type) const auto codeHstring{ winrt::to_hstring(cmark_node_get_literal(node)) }; // The literal for a code node always includes the trailing newline. // Trim that off. - std::wstring_view codeView{ codeHstring.c_str(), codeHstring.size() - 1 }; + const std::wstring_view codeView{ codeHstring.c_str(), codeHstring.size() - 1 }; auto codeBlock = winrt::make(winrt::hstring{ codeView }); WUX::Documents::InlineUIContainer codeContainer{}; @@ -282,7 +282,7 @@ void MarkdownToXaml::_RenderNode(cmark_node* node, cmark_event_type ev_type) cmark_node* parent = cmark_node_parent(node); cmark_node* grandparent = cmark_node_parent(parent); - if (grandparent != NULL && cmark_node_get_type(grandparent)) + if (grandparent != nullptr && cmark_node_get_type(grandparent)) { tight = cmark_node_get_list_tight(grandparent); } diff --git a/src/cascadia/UIMarkdown/MarkdownToXaml.h b/src/cascadia/UIMarkdown/MarkdownToXaml.h index d5e3c71dc38..582d54733eb 100644 --- a/src/cascadia/UIMarkdown/MarkdownToXaml.h +++ b/src/cascadia/UIMarkdown/MarkdownToXaml.h @@ -27,9 +27,9 @@ struct MarkdownToXaml winrt::Windows::UI::Xaml::Documents::Run _CurrentRun(); winrt::Windows::UI::Xaml::Documents::Span _CurrentSpan(); winrt::Windows::UI::Xaml::Documents::Run _NewRun(); - void _EndRun(); - void _EndSpan(); - void _EndParagraph(); + void _EndRun() noexcept; + void _EndSpan() noexcept; + void _EndParagraph() noexcept; winrt::Windows::UI::Xaml::Controls::TextBlock _makeDefaultTextBlock(); diff --git a/src/cascadia/WinRTUtils/inc/ThrottledFunc.h b/src/cascadia/WinRTUtils/inc/ThrottledFunc.h index 40482faf192..94c42b3362a 100644 --- a/src/cascadia/WinRTUtils/inc/ThrottledFunc.h +++ b/src/cascadia/WinRTUtils/inc/ThrottledFunc.h @@ -113,7 +113,7 @@ class ThrottledFunc : public std::enable_shared_from_this_func, self->_storage.take()); + self->_storage.apply(self->_func); } CATCH_LOG(); } diff --git a/src/cascadia/inc/cppwinrt_utils.h b/src/cascadia/inc/cppwinrt_utils.h index 8276b7325e6..9840e9532ba 100644 --- a/src/cascadia/inc/cppwinrt_utils.h +++ b/src/cascadia/inc/cppwinrt_utils.h @@ -208,7 +208,7 @@ public: \ protected: \ type _##name{ __VA_ARGS__ }; \ - void _set##name(const type& value) \ + void _set##name(const type& value) noexcept(noexcept(_##name = value)) \ { \ _##name = value; \ }; diff --git a/src/host/input.cpp b/src/host/input.cpp index fd92154020c..952d3a02915 100644 --- a/src/host/input.cpp +++ b/src/host/input.cpp @@ -321,28 +321,35 @@ void ProcessCtrlEvents() return; } - auto Status = STATUS_SUCCESS; + const auto ctrl = ServiceLocator::LocateConsoleControl(); + for (const auto& r : termRecords) { - /* - * Status will be non-successful if a process attached to this console - * vetoes shutdown. In that case, we don't want to try to kill any more - * processes, but we do need to make sure we continue looping so we - * can close any remaining process handles. The exception is if the - * process is inaccessible, such that we can't even open a handle for - * query. In this case, use best effort to send the close event but - * ignore any errors. - */ - if (SUCCEEDED_NTSTATUS(Status)) - { - Status = ServiceLocator::LocateConsoleControl() - ->EndTask(r.dwProcessID, - EventType, - CtrlFlags); - if (!r.hProcess) - { - Status = STATUS_SUCCESS; - } - } + // Older versions of Windows would do various things if the EndTask() call failed: + // * XP: Pops up a "Windows can't end this program" dialog for every already-dead process. + // * Vista - Win 7: Simply skips over already-dead processes. + // * Win 8 - Win 11 26100: Aborts on an already-dead process (you have to WM_CLOSE conhost multiple times). + // + // That last period had the following comment: + // Status will be non-successful if a process attached to this console + // vetoes shutdown. In that case, we don't want to try to kill any more + // processes, but we do need to make sure we continue looping so we + // can close any remaining process handles. The exception is if the + // process is inaccessible, such that we can't even open a handle for + // query. In this case, use best effort to send the close event but + // ignore any errors. + // + // The corresponding logic worked like this: + // if (FAILED(EndTask(...)) && r.hProcess) { + // break; + // } + // + // That logic was removed around the Windows 11 26100 time frame, because CSRSS + // (which handles EndTask) now waits 5s and then force-kills the process for us. + // Going back to the Win 7 behavior then should make shutdown a lot more robust. + // The bad news is that EndTask() returns STATUS_UNSUCCESSFUL no matter whether + // the process was already dead, or if the request actually failed for some reason. + // Hopefully there aren't any regressions, but we can't know without trying. + LOG_IF_NTSTATUS_FAILED(ctrl->EndTask(r.dwProcessID, EventType, CtrlFlags)); } } diff --git a/src/host/settings.cpp b/src/host/settings.cpp index 746bd91564f..4650fbf9eb1 100644 --- a/src/host/settings.cpp +++ b/src/host/settings.cpp @@ -36,7 +36,7 @@ Settings::Settings() : _bAutoPosition(true), _uHistoryBufferSize(DEFAULT_NUMBER_OF_COMMANDS), _uNumberOfHistoryBuffers(DEFAULT_NUMBER_OF_BUFFERS), - _bHistoryNoDup(true), + _bHistoryNoDup(false), // ColorTable initialized below _uCodePage(ServiceLocator::LocateGlobals().uiOEMCP), _uScrollScale(1), @@ -110,7 +110,7 @@ void Settings::ApplyDesktopSpecificDefaults() _bQuickEdit = TRUE; _uHistoryBufferSize = 50; _uNumberOfHistoryBuffers = 4; - _bHistoryNoDup = true; + _bHistoryNoDup = FALSE; _renderSettings.ResetColorTable(); diff --git a/src/inc/til/throttled_func.h b/src/inc/til/throttled_func.h index d250f475af5..7a2d544add3 100644 --- a/src/inc/til/throttled_func.h +++ b/src/inc/til/throttled_func.h @@ -30,12 +30,22 @@ namespace til } } - std::tuple take() + void apply(const auto& func) { - std::unique_lock guard{ _lock }; - auto pendingRunArgs = std::move(*_pendingRunArgs); - _pendingRunArgs.reset(); - return pendingRunArgs; + decltype(_pendingRunArgs) args; + { + std::unique_lock guard{ _lock }; + args = std::exchange(_pendingRunArgs, std::nullopt); + } + // Theoretically it should always have a value, because the throttled_func + // should not call the callback without there being a reason. + // But in practice a failure here was observed at least once. + // It's unknown to me what caused it, so the best we can do is avoid a crash. + assert(args.has_value()); + if (args) + { + std::apply(func, *args); + } } explicit operator bool() const @@ -60,10 +70,12 @@ namespace til return _isPending.exchange(true, std::memory_order_relaxed); } - std::tuple<> take() + void apply(const auto& func) { - reset(); - return {}; + if (_isPending.exchange(false, std::memory_order_relaxed)) + { + func(); + } } void reset() @@ -171,31 +183,24 @@ namespace til void flush() { WaitForThreadpoolTimerCallbacks(_timer.get(), true); - if (_storage) - { - _trailing_edge(); - } + _timer_callback(nullptr, this, nullptr); } private: static void __stdcall _timer_callback(PTP_CALLBACK_INSTANCE /*instance*/, PVOID context, PTP_TIMER /*timer*/) noexcept try { - static_cast(context)->_trailing_edge(); - } - CATCH_LOG() - - void _trailing_edge() - { + const auto self = static_cast(context); if constexpr (Leading) { - _storage.reset(); + self->_storage.reset(); } else { - std::apply(_func, _storage.take()); + self->_storage.apply(self->_func); } } + CATCH_LOG() wil::unique_threadpool_timer _createTimer() { diff --git a/src/inc/til/winrt.h b/src/inc/til/winrt.h index c5e68bac776..101d558f7f9 100644 --- a/src/inc/til/winrt.h +++ b/src/inc/til/winrt.h @@ -8,12 +8,12 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" template struct property { - explicit constexpr property(auto&&... args) : + explicit constexpr property(auto&&... args) noexcept(std::is_nothrow_constructible_v) : _value{ std::forward(args)... } {} property& operator=(const property& other) = default; - T operator()() const noexcept + T operator()() const noexcept(std::is_nothrow_copy_constructible::value) { return _value; } diff --git a/src/propsheet/registry.cpp b/src/propsheet/registry.cpp index 3a7283e080e..64d392aa76d 100644 --- a/src/propsheet/registry.cpp +++ b/src/propsheet/registry.cpp @@ -79,7 +79,7 @@ VOID InitRegistryValues( pStateInfo->CursorSize = 25; pStateInfo->HistoryBufferSize = 25; pStateInfo->NumberOfHistoryBuffers = 4; - pStateInfo->HistoryNoDup = 1; + pStateInfo->HistoryNoDup = 0; // clang-format off if (pStateInfo->fIsV2Console) diff --git a/src/server/ProcessList.cpp b/src/server/ProcessList.cpp index e304556692a..e01e20bb8de 100644 --- a/src/server/ProcessList.cpp +++ b/src/server/ProcessList.cpp @@ -211,9 +211,15 @@ ConsoleProcessHandle* ConsoleProcessList::GetOldestProcess() const { termRecords.clear(); + // The caller (ProcessCtrlEvents) expects them in newest-to-oldest order, + // because that's how Windows has historically always dispatched these events. + auto it = _processes.crbegin(); + const auto end = _processes.crend(); + // Dig through known processes looking for a match - for (const auto& p : _processes) + for (; it != end; ++it) { + const auto p = *it; // If no limit was specified OR if we have a match, generate a new termination record. if (!dwLimitingProcessId || p->_ulProcessGroupId == dwLimitingProcessId)