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

"Run" sees incorrect version info ("1903+" on "2004") #4432

Closed
GeorgeSpelvin opened this issue Jun 22, 2020 · 11 comments
Closed

"Run" sees incorrect version info ("1903+" on "2004") #4432

GeorgeSpelvin opened this issue Jun 22, 2020 · 11 comments
Assignees
Labels
Issue-Bug Something isn't working Product-PowerToys Run Improved app launch PT Run (Win+R) Window Product-Settings The standalone PowerToys Settings application Resolution-Fix Committed Fix is checked in, but it might be 3-4 weeks until a release.
Milestone

Comments

@GeorgeSpelvin
Copy link

Environment

Windows build number:  2004 (Microsoft Windows [Version 10.0.19041.329])
PowerToys version:  0.18.2
PowerToy module for which you are reporting the bug (if applicable):  RUN

Steps to reproduce

  1. Have Windows 10 v 2004.
  2. Look in Settings...Run.

Expected behavior

Run no longer configurable because it thinks I have an old version of Windows.

Actual behavior

"This feature requires Windows 10 version 1903 or higher"

@ghost ghost added the Needs-Triage For issues raised to be triaged and prioritized by internal Microsoft teams label Jun 22, 2020
@crutkas
Copy link
Member

crutkas commented Jun 23, 2020

Can you take a screenshot of your settings?

this looks to be an OS detection bug. @arjunbalgovind / @enricogior did #4253 fix this issue? This looks to be settings related as well if they are pulling out old settings.

@crutkas crutkas added Issue-Bug Something isn't working Product-PowerToys Run Improved app launch PT Run (Win+R) Window Product-Settings The standalone PowerToys Settings application Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something and removed Needs-Triage For issues raised to be triaged and prioritized by internal Microsoft teams labels Jun 23, 2020
@arjunbalgovind
Copy link
Contributor

No this isn't fixed. #4253 only prevented PowerLauncher.exe from starting up on older OS versions. This seems to be the same issue reported in #3925 , it looks like right after the updating to 2004 users are hitting this, but after "some reboots" (I'm not sure if anything else changed in between this for users), it starts working again. It sounds like maybe the updated "hasn't completed"? I tried to reproduce this on a VM by upgrading an 1809 VM to 2004 but I couldn't repro it.

@enricogior
Copy link
Contributor

I'm still thinking we should just check the registry since it's more robust and can't fail as the current approach.

@crutkas
Copy link
Member

crutkas commented Jun 23, 2020

@enricogior You mean detect contracts VS the WinVer?

@enricogior
Copy link
Contributor

@crutkas
yes, since in our scenario we don't really need to check for the API (we know those APIs will never be supported on 1809 and older)

@ghost ghost added the Status-No recent activity no activity in the past 5 days when follow up's are needed label Jun 29, 2020
@ghost
Copy link

ghost commented Jun 29, 2020

This issue has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 5 days. It will be closed if no further activity occurs within 5 days of this comment.

@DefaultRyan
Copy link
Member

Is this bug reproducible? The code for IsApiContractPresent hasn't seen many changes lately (over 1 year, in fact), so I'd be surprised if this suddenly stopped working. There's also nothing that I'm immediately seeing in that API that is stateful, which confirms that I haven't ever seen this API behave differently from call to call. Either way, I'd like to see some debugging/tracing showing that the API is failing or returning the wrong value.

I'm wondering if there is any threading going on in the calling side? UseNewSettings is not thread safe right now:

template<uint16_t APIVersion>
bool IsAPIContractVxAvailable()
{
    static bool isAPIContractVxAvailableInitialized = false;
    static bool isAPIContractVxAvailable = false;
    if (!isAPIContractVxAvailableInitialized)
    {
        isAPIContractVxAvailableInitialized = true; // This is vulnerable to data races.
        isAPIContractVxAvailable = winrt::Windows::Foundation::Metadata::ApiInformation::IsApiContractPresent(L"Windows.Foundation.UniversalApiContract", APIVersion);
    }

    return isAPIContractVxAvailable;
}

So that's kind of my hunch that could explain the inconsistent behavior. This would be far more reliable, not to mention simpler:

template<uint16_t APIVersion>
bool IsAPIContractVxAvailable()
{
    static const bool isAPIContractVxAvailable = winrt::Windows::Foundation::Metadata::ApiInformation::IsApiContractPresent(L"Windows.Foundation.UniversalApiContract", APIVersion);

    return isAPIContractVxAvailable;
}

Simply rely on C++'s thread-safe local static initialization to do the job.

@ghost ghost removed the Status-No recent activity no activity in the past 5 days when follow up's are needed label Jun 29, 2020
@enricogior
Copy link
Contributor

Hi @DefaultRyan
thanks for the suggestion, we'll adopt it.

@alekhyareddy28
Copy link
Contributor

alekhyareddy28 commented Jun 29, 2020

Thanks @DefaultRyan. @enricogior /@arjunbalgovind, it seems that that might be the issue because looking at all the os detection issues (other than the PowerLauncher issue, which has been fixed), it seems like they are all happening when an older version shows up on recent builds. This might be because the isAPIContractVxAvailableInitialized is being set to true in one thread and it is not detecting the os in the other process/thread as it is no longer executing the code inside the if statement.

@crutkas
Copy link
Member

crutkas commented Jun 29, 2020

image
make it so team

@alekhyareddy28 alekhyareddy28 self-assigned this Jul 2, 2020
@alekhyareddy28 alekhyareddy28 added the Status-In progress This issue or work-item is under development label Jul 2, 2020
@alekhyareddy28 alekhyareddy28 added this to the InVEST-2007 milestone Jul 2, 2020
@alekhyareddy28 alekhyareddy28 added Resolution-Fix Committed Fix is checked in, but it might be 3-4 weeks until a release. and removed Status-In progress This issue or work-item is under development Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something labels Jul 3, 2020
@crutkas
Copy link
Member

crutkas commented Jul 7, 2020

@crutkas crutkas closed this as completed Jul 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Issue-Bug Something isn't working Product-PowerToys Run Improved app launch PT Run (Win+R) Window Product-Settings The standalone PowerToys Settings application Resolution-Fix Committed Fix is checked in, but it might be 3-4 weeks until a release.
Projects
None yet
Development

No branches or pull requests

6 participants