Add COM server idle timeout for cleaner MSIX upgrades#22
Conversation
When all Terminal windows are closed AND all COM client objects are released, start a 5-second grace-period timer. If the timer fires and the process is still idle, post WM_QUIT to exit cleanly. This prevents WindowsTerminal.exe from lingering indefinitely after the last window closes (due to COM server registration keeping the process alive), which blocks MSIX package upgrades with error 0x80073D02. Design: - Track live COM objects via atomic counter (constructor/destructor) - COM MTA thread posts WM_COM_IDLE_CHECK to emperor's UI thread - _updateComIdleTimer() recomputes state statelessly each time - WM_TIMER fires -> double-checks conditions -> PostQuitMessage - New window or COM client cancels the timer - Non-headless quit path is completely unchanged Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR adds an “idle shutdown” mechanism to Windows Terminal’s out-of-proc COM server path so the process exits shortly after becoming truly idle (no windows, no message boxes, and no connected COM clients), preventing MSIX upgrade failures caused by a lingering WindowsTerminal.exe.
Changes:
- Track live COM server object instances via an atomic counter and notify the UI thread when the count changes.
- Add a COM-idle grace-period timer in
WindowEmperorthat postsWM_QUITafter 5 seconds of sustained idleness. - Wire up the emperor message-window
HWNDso COM MTA threads canPostMessagean idle-check request to the UI thread.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/cascadia/WindowsTerminal/WindowEmperor.h | Adds WM_COM_IDLE_CHECK message and COM-idle timer constants + helper declaration. |
| src/cascadia/WindowsTerminal/WindowEmperor.cpp | Implements COM idle timer recomputation and message handling; sets/clears emperor HWND for COM thread notifications. |
| src/cascadia/WindowsTerminal/TerminalProtocolComServer.h | Adds constructor and static state for emperor HWND + live object counter access. |
| src/cascadia/WindowsTerminal/TerminalProtocolComServer.cpp | Implements live object counting and cross-thread idle-check posting. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
This comment has been minimized.
This comment has been minimized.
- Make s_emperorHwnd std::atomic<HWND> to fix data race between UI thread (writer) and COM MTA thread (reader) - Add AllowHeadless() check to idle timer — headless mode stays alive indefinitely, matching _postQuitMessageIfNeeded behavior - Add IDT to spelling expect list Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
src/cascadia/WindowsTerminal/TerminalProtocolComServer.cpp:131
- Same as in the constructor: the decrement side of the liveness counter doesn’t need release semantics (and the reader doesn’t need acquire) since no other state is being published/consumed via this atomic. Consider using
memory_order_relaxedto keep teardown fast and consistent with the intended use as a plain counter.
TerminalProtocolComServer::~TerminalProtocolComServer()
{
_removeInstance();
s_liveObjectCount.fetch_sub(1, std::memory_order_release);
s_notifyEmperorIdleCheck();
}
- Update static state comment to distinguish immutable (s_emperor) from mutable (s_emperorHwnd, s_liveObjectCount) members - Switch s_liveObjectCount to memory_order_relaxed since it's a pure numeric liveness counter with no data dependencies Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
|
@PankajBhojwani , this is the COMServer will not shutdown itself when no usage one. |
Resolve merge conflicts: - TerminalProtocolComServer.h: keep both s_GetLiveObjectCount and s_OnWindowAdded - WindowEmperor.cpp: keep both idle timer update and s_OnWindowAdded call - expect.txt: merge both sets of spelling entries Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
The function accessed private static member s_emperorHwnd but was defined as a file-level static instead of a class static method. Move it to TerminalProtocolComServer:: scope and declare in header. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
_postQuitMessageIfNeeded now checks s_liveObjectCount so the process
does not exit immediately when windows close while COM clients are
still connected.
_updateComIdleTimer starts a grace-period timer whenever the process
becomes headless (no windows, no message boxes):
- If no COM objects remain: 5 s (COM_IDLE_TIMEOUT_MS).
- If COM objects still exist: 30 s (COM_STALE_TIMEOUT_MS) to cover
crashed clients whose stub references are stuck in COM GC.
When the timer fires the process exits regardless of outstanding COM
objects -- any survivors at that point are stale stubs that will never
be reclaimed. TerminateProcess (after the message loop) cleans up.
WM_COM_IDLE_CHECK now also calls _postQuitMessageIfNeeded so that a
clean COM disconnect triggers an immediate exit instead of waiting for
the timer.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
When multiple COM clients are killed, COM GC releases stubs one at a time. Each release triggered _updateComIdleTimer() which called SetTimer() again, restarting the 30s countdown. With 3 staggered releases this could extend the total wait to 90s+. Track the currently active timer value in _activeComIdleTimeoutMs and skip SetTimer() if the desired timeout hasn't changed. Reset to 0 when the timer fires or is killed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
- Fix _postQuitMessageIfNeeded comment to mention COM object count check - Document why WM_TIMER intentionally skips s_GetLiveObjectCount: stale timer overrides crashed-client stubs; legitimate new connections create windows which are checked via _windowCount Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@check-spelling-bot Report
|
| Dictionary | Entries | Covers | Uniquely |
|---|---|---|---|
| cspell:csharp/csharp.txt | 32 | 2 | 2 |
| cspell:aws/aws.txt | 232 | 2 | 2 |
| cspell:fonts/fonts.txt | 536 | 1 | 1 |
Consider adding to the extra_dictionaries array (in the .github/actions/spelling/config.json file):
"cspell:csharp/csharp.txt",
"cspell:aws/aws.txt",
"cspell:fonts/fonts.txt",
To stop checking additional dictionaries, put (in the .github/actions/spelling/config.json file):
"check_extra_dictionaries": []Forbidden patterns 🙅 (9)
In order to address this, you could change the content to not match the forbidden patterns (comments before forbidden patterns may help explain why they're forbidden), add patterns for acceptable instances, or adjust the forbidden patterns themselves.
These forbidden patterns matched content:
Should be nonexistent
\b[Nn]o[nt][- ]existent\b
Should be preexisting
[Pp]re[- ]existing
Should be ; otherwise or . Otherwise
https://study.com/learn/lesson/otherwise-in-a-sentence.html
, [Oo]therwise\b
Should probably be Otherwise,
(?<=\. )Otherwise\s
Complete sentences in parentheticals should not have a space before the period.
\s\.\)(?!.*\}\})
Should be set up (setup is a noun / set up is a verb)
\b[Ss]etup(?= (?:an?|the|to)\b)
Should be reentrant
[Rr]e[- ]entrant
Should be whether or not ...
(?i)\b(?:whe|ra)ther(?:\s\w+)+ or not\.
Should be WinGet
\bWinget\b
Pattern suggestions ✂️ (1)
You could add these patterns to .github/actions/spelling/patterns/75ba38412d70c1019a205fe7ef6e2ef4ec9ce694.txt:
# Automatically suggested patterns
# hit-count: 1 file-count: 1
# python
\b(?i)py(?!gment|gmy|lon|ramid|ro|th)(?=[a-z]{2,})
Alternatively, if a pattern suggestion doesn't make sense for this project, add a # to the beginning of the line in the candidates file with the pattern to stop suggesting it.
Errors, Warnings, and Notices ❌ (9)
See the 📂 files view, the 📜action log, 👼 SARIF report, or 📝 job summary for details.
See ❌ Event descriptions for more information.
✏️ Contributor please read this
By default the command suggestion will generate a file named based on your commit. That's generally ok as long as you add the file to your commit. Someone can reorganize it later.
If the listed items are:
- ... misspelled, then please correct them instead of using the command.
- ... names, please add them to
.github/actions/spelling/allow/names.txt. - ... APIs, you can add them to a file in
.github/actions/spelling/allow/. - ... just things you're using, please add them to an appropriate file in
.github/actions/spelling/expect/. - ... tokens you only need in one place and shouldn't generally be used, you can add an item in an appropriate file in
.github/actions/spelling/patterns/.
See the README.md in each directory for more information.
🔬 You can test your commits without appending to a PR by creating a new branch with that extra change and pushing it to your fork. The check-spelling action will run in response to your push -- it doesn't require an open pull request. By using such a branch, you can limit the number of typos your peers see you make. 😉
If the flagged items are 🤯 false positives
If items relate to a ...
-
binary file (or some other file you wouldn't want to check at all).
Please add a file path to the
excludes.txtfile matching the containing file.File paths are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your files.
^refers to the file's path from the root of the repository, so^README\.md$would exclude README.md (on whichever branch you're using). -
well-formed pattern.
If you can write a pattern that would match it,
try adding it to thepatterns.txtfile.Patterns are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your lines.
Note that patterns can't match multiline strings.
| const auto headless = | ||
| _windowCount <= 0 && | ||
| _messageBoxCount <= 0 && | ||
| !_app.Logic().Settings().GlobalSettings().AllowHeadless(); | ||
|
|
||
| if (headless) | ||
| { | ||
| const auto timeout = TerminalProtocolComServer::s_GetLiveObjectCount() > 0 | ||
| ? COM_STALE_TIMEOUT_MS | ||
| : COM_IDLE_TIMEOUT_MS; | ||
| if (_activeComIdleTimeoutMs != timeout) |
| // Any remaining COM objects belong to crashed clients whose | ||
| // stub references haven't been reclaimed by the COM GC yet | ||
| // (COM GC can take up to 6+ minutes for killed processes). | ||
| // We intentionally do NOT check s_GetLiveObjectCount here — | ||
| // the stale timer exists precisely to override those stubs. | ||
| // A legitimate new connection during this window would have | ||
| // created a window (_windowCount > 0), which is checked below. |
| CLDR | ||
| doy | ||
| Hinnant | ||
| IDT |
| const auto headless = | ||
| _windowCount <= 0 && | ||
| _messageBoxCount <= 0 && | ||
| !_app.Logic().Settings().GlobalSettings().AllowHeadless(); | ||
|
|
||
| if (headless) | ||
| { | ||
| const auto timeout = TerminalProtocolComServer::s_GetLiveObjectCount() > 0 | ||
| ? COM_STALE_TIMEOUT_MS | ||
| : COM_IDLE_TIMEOUT_MS; | ||
| if (_activeComIdleTimeoutMs != timeout) |
| // Any remaining COM objects belong to crashed clients whose | ||
| // stub references haven't been reclaimed by the COM GC yet | ||
| // (COM GC can take up to 6+ minutes for killed processes). | ||
| // We intentionally do NOT check s_GetLiveObjectCount here — | ||
| // the stale timer exists precisely to override those stubs. | ||
| // A legitimate new connection during this window would have | ||
| // created a window (_windowCount > 0), which is checked below. |
| CLDR | ||
| doy | ||
| Hinnant | ||
| IDT |
Problem
When all Terminal windows are closed, WindowsTerminal.exe can linger indefinitely because the COM server registration (TerminalProtocolComServer) keeps the process alive. This blocks MSIX package upgrades with error 0x80073D02.
Solution
Add a 5-second idle timeout: when the process has no windows, no message boxes, and no live COM client objects, start a grace-period timer. If the timer fires and the process is still idle, post \WM_QUIT\ to exit cleanly.
Design
What's unchanged
The existing non-headless quit path (_postQuitMessageIfNeeded) is completely unchanged. The idle timer is an additional mechanism that only engages when the process would otherwise linger.
Files changed
Testing