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

Should remove synchronous block from LaunchSettingsProvider #1342

Closed
BillHiebert opened this issue Jan 26, 2017 · 9 comments · Fixed by #2317
Closed

Should remove synchronous block from LaunchSettingsProvider #1342

BillHiebert opened this issue Jan 26, 2017 · 9 comments · Fixed by #2317
Assignees
Labels
Blocking-Partner A partner is blocked from implementing an important bug or feature by this issue.
Milestone

Comments

@BillHiebert
Copy link
Contributor

Using synchronous calls within async blocks can cause threadpool exhausting. Should investigate removing the ExecuteSynchronously call in the code below:

        private string _launchSettingsFileFolder;
        private string LaunchSettingsFileFolder
        {
            get
            {
                if (_launchSettingsFileFolder == null)
                {
                    _launchSettingsFileFolder = CommonProjectServices.ThreadingService.ExecuteSynchronously(async () => {
                        var props = await CommonProjectServices.ActiveConfiguredProjectProperties.GetAppDesignerPropertiesAsync().ConfigureAwait(true);
                        return await props.FolderName.GetValueAsync().ConfigureAwait(true) as string;
                    });

                    if (string.IsNullOrEmpty(_launchSettingsFileFolder))
                    {
                        _launchSettingsFileFolder = DefaultSettingsFileFolder;
                    }
                }
                return _launchSettingsFileFolder;
            }
        }
@BillHiebert
Copy link
Contributor Author

Note. Should import the ISpecialFilesManager and use that to get the app designer folder.

@srivatsn srivatsn added the Bug label Jan 30, 2017
@srivatsn srivatsn added this to the 1.1 milestone Jan 30, 2017
@srivatsn
Copy link
Contributor

We're unlikely to change this now and it's not causing any issues.

@davkean davkean reopened this May 18, 2017
@davkean
Copy link
Member

davkean commented May 18, 2017

I'm debugging through Roslyn right now and I'm seeing signs that this is causing issues, I see 53 threads waiting just on this:

image

@davkean davkean added the Blocking-Partner A partner is blocked from implementing an important bug or feature by this issue. label May 18, 2017
@davkean
Copy link
Member

davkean commented May 18, 2017

It looks like the majority of entry points into LaunchSettingsProvider are async, so we should just flow this through.

@lifengl
Copy link
Contributor

lifengl commented May 19, 2017

Generally, we should prevent using JTF.Run other than implementing old contracts (especially old COM/UI thread contracts). Using it inside a thread pool thread may cause thread pool exhaustion issues, and froze the process for half second without CPU activities. Sometime, it takes two threads in JTF.Run to trigger this problem. We saw this issue in Dev 14, and tried to reduce them in CPS.

The reason is if you have code to block on thread pool and wait other async method to finish. Very often, the async method may require to run code in the thread pool to finish, so when the number of threads in this state reaches the number of thread pool threads, it will enter a virtual dead lock mode, until the thread pool manager to kick in and decide to add new threads to the pool. Thread pool manager only wakes up maybe every 250ms, we typically see it unblocked the pool after 500ms in dev 14. And we notice this type of issue randomly freezes IDE for short period of time, and very hard to reproduce.

@davkean
Copy link
Member

davkean commented May 19, 2017

Yeah I'm seeing random freezes - I wonder if this is related.

@lifengl
Copy link
Contributor

lifengl commented May 19, 2017 via email

@davkean
Copy link
Member

davkean commented May 30, 2017

Just ran into the same thing opening ProjectSystem.sln, just sitting there blocked on UI thread without zero CPU. I don't have a trace.
image

@davkean
Copy link
Member

davkean commented May 30, 2017

I have a proposed fix here: #2317. I'm yet to run this through the mill yet - I plan on doing this tomorrow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Blocking-Partner A partner is blocked from implementing an important bug or feature by this issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants