-
Notifications
You must be signed in to change notification settings - Fork 1.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Coordinated fix for capturing continuation context for tasks in Desktop apps #2654
Coordinated fix for capturing continuation context for tasks in Desktop apps #2654
Conversation
For reference, this is a mirror of internal MSVC-PR-339372 (and should be binary-identical). |
Where are the |
No, these flags are not documented. They're not part of ConcRT or UCRT - they're exclusive to PPL (and the OS and public STL libs). The PPL is deprecated - this fix is to unblock partners who are moving CX code from UWP to Desktop execution. There is no ODR concern with the #ifdefs around these symbols. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @barcharcraz for reviewing. As there were several conversations running in parallel, I wanted to summarize the changes that I believe are necessary:
- Remove
"delayimp.lib"
fromtarget_link_libraries
- Remove
"-delayload:ole32.dll"
fromtarget_link_options
(i.e. reverting that line entirely) - Corresponding changes to the MSVC-internal build system
With delay-loading removed, and as long as this doesn't result in COM being initialized for anyone who wasn't already initializing COM (I think it's safe, looking at the code being enabled here), then I think I'll be comfortable with the change even though I'm far from understanding everything that's involved here.
…ly, abandoning Vista support
Thanks for fixing this bug, and congratulations on your first microsoft/STL commit! 🎉 🐞 😸 |
…in Desktop apps (microsoft#2654)" This reverts commit 80ebe87.
With the adoption of WinUI3 (via Windows App SDK), Desktop Xaml apps are now supported. However, ppltasks client code in a Desktop app would fail to capture the apartment context for continuation callbacks, by default (globally, without needing to pass an explicit task_continuation_context to every continuation). This PR fixes that bug.
For C++/CX client code (which may be in process of migration to Desktop C++/WinRT code), no other changes are needed.
For ISO C++ client code, this fix builds on work to recover ppltasks functionality lost during the UCRT migration (see https://www.osgwiki.com/wiki/Universal_CRT#PPL_Tasks for details), which requires adding
#define PPL_TASK_CONTEXT_ERROR_CONTROL 1
to opt into using context callbacks and error reporting (UWP behavior).
Note that C++/CLI does not support ppltasks.h: fatal error C1189: #error: <condition_variable> is not supported when compiling with /clr or /clr:pure
If previous behavior is desired (callbacks on an arbitrary continuation thread), code can add
#define PPL_TASK_CONTEXT_DEFAULT_ARBITRARY 1
C++/CX code can also explicitly pass concurrency::task_continuation_context::use_arbitrary() to then().
The fix requires CoGetApartmentType and CoGetObjectContext, in turn requiring Windows 7 or later. This dependency is handled by a #pragma comment ( lib, "ole32" ) in ppltasks.cpp. In addition, msvcp140.dll is built with delay-loaded ole32, so that applications can continue to be deployed to Vista, if this code is not used. Support for OneCore SKUs is assumed to be handled by ApiSet reverse-forwarding from the ole32.dll linkage. No compile time checks against WINVER/_WIN32_WINNT have been added, as the docs officially state Windows 7 as the base supported version for Visual Studio.