Add get_only_safe_from_non_presenting_sta() to async operations#1565
Open
ChrisGuzak wants to merge 1 commit into
Open
Add get_only_safe_from_non_presenting_sta() to async operations#1565ChrisGuzak wants to merge 1 commit into
ChrisGuzak wants to merge 1 commit into
Conversation
Add a peer to .get() on IAsyncAction, IAsyncOperation, IAsyncActionWithProgress, and IAsyncOperationWithProgress that skips the _DEBUG-only STA blocking assert. The existing .get() asserts !is_sta_thread() to guard against blocking UI threads. However, not all STAs are UI threads — some never present UI, haven't presented yet, or never will. The assert is also _DEBUG-only, making it invisible to codebases that don't build with _DEBUG (e.g. the Windows OS). The new method get_only_safe_from_non_presenting_sta() is functionally identical to .get() but omits the STA check. The intentionally long name communicates the risk to callers. Changes: - strings/base_coroutine_foundation.h: Add wait_get_bypass_sta_check() impl helper and get_only_safe_from_non_presenting_sta() for all 4 async consume templates - cppwinrt/code_writers.h: Add declaration to generated code for all 4 async types - test/test_nocoro: Add test calling the new method from an STA thread using a real WinRT async operation (PathIO::ReadTextAsync on C:\Windows\win.ini) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Member
|
Not the hugest fan of the very_long_function_name but I don't have a better answer quite yet. Things that come to mind:
|
Contributor
|
I think get_unchecked() is probably the best one, it immediately flags to the reader this requires extra carefulness while not being overly verbose. |
Contributor
|
How about naming it |
There was a problem hiding this comment.
Pull request overview
This PR adds a new synchronous “get-like” API across the four WinRT async interfaces that intentionally bypasses the _DEBUG-only STA blocking assert, enabling blocking waits from STAs that are not presenting UI.
Changes:
- Added
get_only_safe_from_non_presenting_sta()to the consume extensions forIAsyncAction,IAsyncOperation<T>,IAsyncActionWithProgress<T>, andIAsyncOperationWithProgress<T, P>. - Added an internal helper
impl::wait_get_bypass_sta_check()mirroring the existing.get()wait/get behavior but skipping the STA assert. - Added a
test_nocorotest that invokes the new method from an STA thread usingWindows::Storage::PathIO::ReadTextAsync.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| test/test_nocoro/pch.h | Adds Windows.Storage include to support the new test’s WinRT API usage. |
| test/test_nocoro/get.cpp | Adds an STA-threaded test case exercising get_only_safe_from_non_presenting_sta(). |
| strings/base_coroutine_foundation.h | Implements the bypass helper and wires the new method into async consume templates. |
| cppwinrt/code_writers.h | Updates generated interface extension declarations to include the new method. |
Comment on lines
+83
to
+92
| std::thread sta_thread([&failure] | ||
| { | ||
| try | ||
| { | ||
| winrt::init_apartment(winrt::apartment_type::single_threaded); | ||
|
|
||
| auto content = PathIO::ReadTextAsync(L"C:\\Windows\\win.ini").get_only_safe_from_non_presenting_sta(); | ||
|
|
||
| REQUIRE(content.size() > 0); | ||
|
|
Comment on lines
+88
to
+93
|
|
||
| auto content = PathIO::ReadTextAsync(L"C:\\Windows\\win.ini").get_only_safe_from_non_presenting_sta(); | ||
|
|
||
| REQUIRE(content.size() > 0); | ||
|
|
||
| winrt::uninit_apartment(); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Add a peer to
.get()on all four WinRT async interfaces (IAsyncAction,IAsyncOperation,IAsyncActionWithProgress,IAsyncOperationWithProgress) that skips the_DEBUG-only STA blocking assert.Motivation
The existing
.get()asserts!is_sta_thread()to guard against blocking UI threads. However:_DEBUG-only (WINRT_ASSERTis a no-op in release), making it invisible to codebases that don't build with_DEBUG(e.g. the Windows OS). This means the protection is already weak.The new method
get_only_safe_from_non_presenting_sta()is functionally identical to.get()but omits the STA check. The intentionally long name communicates the risk to callers.Changes
strings/base_coroutine_foundation.h: Addwait_get_bypass_sta_check()impl helper andget_only_safe_from_non_presenting_sta()definitions for all 4 async consume templatescppwinrt/code_writers.h: Add declaration to code-generated output for all 4 async typestest/test_nocoro: Add test that calls the new method from an STA thread using a real WinRT async operation (PathIO::ReadTextAsynconC:\Windows\win.ini)Testing
Full solution builds clean.
test_nocoropasses (2 assertions in 2 test cases), including the new STA-based test.