Skip to content
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

Revert the ppltasks change that introduced an ole32.dll dependency #3607

Merged
merged 2 commits into from
Apr 4, 2023

Conversation

StephanTLavavej
Copy link
Member

@StephanTLavavej StephanTLavavej commented Mar 31, 2023

This is a clean revert (no manual changes) of #2654 and the following #3255 (in reverse order). #3255 was a targeted fix (partial revert) for an incredibly disruptive bincompat bug, but #2654 has caused even more trouble. This full revert:

  • Fixes ppltasks.cpp: Breaking change - New OLE32.dll import prevents process mitigations #3257, where the newly added dependency on ole32.dll was disruptive to certain exotic scenarios, and
  • Fixes a "sandboxed process" scenario for the XStore team (reported by @vaboca), also disrupted by ole32.dll.
    • Don't ask me to explain this in any more detail, I don't know what's going on here. 🤯 That's the whole problem with this change - nobody fully understood the implications - and that's why we need to fully revert it, not mess around with it more.

By performing a full revert, we no longer need the partial revert. This returns us to the state that we successfully lived with in VS 2015 through VS 2022 17.2 inclusive.

@Scottj1s, the original author of #2654, agrees with the full revert here, and has confirmed that the original affected customer who needed that change has the ability to use a workaround in perpetuity.

🪞 The mirror of this PR, MSVC-PR-462260, additionally reverts the non-GitHub change in MSVC-PR-339372 affecting <ppltasks.h>.

❕ By changing ppltasks.cpp again, this affects the VCRedist. For the bincompat scenario involving mixing VS 2015, this should be unquestionably safe (the partial revert and the full revert result in the same behavior for the function in question, that's why the partial revert worked). For the question of when we can ship this fix, because it is urgently needed by the XStore team, we are planning to port this fix to VS 2022 17.6 before it reaches General Availability, which will require unlocking its redist, and then that unlocked redist will flow into VS 2022 17.7, so there should be no remaining mix-and-match nightmares (I hope).

I am not performing any other changes or cleanups at this time (I may come back to ppltasks.cpp in the future to add some #endif comments, but no logic changes, I have had a lifetime of being burned here).

I've filed #3606 to capture the vNext comments that are being reverted here.

⚠️ Note to self: Remember to start the 17.6 backport process after merging this.

@StephanTLavavej StephanTLavavej added bug Something isn't working high priority Important! affects redist Results in changes to separately compiled bits labels Mar 31, 2023
@StephanTLavavej StephanTLavavej requested a review from a team as a code owner March 31, 2023 04:07
@Scottj1s
Copy link
Member

Thanks STL. Gives me a case of the Moody Blues to see it go, but agree with the call.

@StephanTLavavej StephanTLavavej merged commit 5d4aa49 into microsoft:main Apr 4, 2023
@StephanTLavavej StephanTLavavej deleted the days-of-future-past branch April 4, 2023 23:42
@tringi
Copy link

tringi commented May 3, 2023

Hey. I wanted to note that our SCADA software is too affected by this issue.
Luckily the vast majority of users don't upgrade vcruntime that often (or at all).
Can we expect 17.6 soon?

@StephanTLavavej
Copy link
Member Author

@tringi As a general rule, we can't talk about release dates before they've been officially announced.

In this case, I think I can get away with saying that as 17.6 Preview 6 is the current Preview release, you can look at our historical patterns (which are fairly consistent although not absolute clockwork) and guess that the production release is coming soon (whereas if we were currently something like Preview 2, it would not be "soon"). I can't be any more specific than that, sorry.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects redist Results in changes to separately compiled bits bug Something isn't working high priority Important!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ppltasks.cpp: Breaking change - New OLE32.dll import prevents process mitigations
4 participants