Nuget.Client should use CPS JoinableTaskFactory when interacting with CPS to prevent deadlocks #4185

Closed
jviau opened this Issue Jan 4, 2017 · 5 comments

Comments

Projects
None yet
5 participants
@jviau

jviau commented Jan 4, 2017

Currently Nuget.Client uses the Shell JoinableTaskFactory when interacting with CPS projects NuGetUIThreadHelper. This is causing deadlocks due to project lock issues. The CPS JTF has knowledge of the project lock and logic to prevent these issues. You can get the CPS version of JTF by importing the IProjectThreadingService from CPS. Let me know if you need assistance in the specifics of getting the IProjectThreadingService.

I am not sure how many deadlocks this is causing, but I have identified at least one when NuGet eventually sets an item property from within an ExecuteAction.

@rrelyea rrelyea added this to the 4.0 RC3 milestone Jan 4, 2017

@rrelyea

This comment has been minimized.

Show comment
Hide comment
@rrelyea

rrelyea Jan 4, 2017

Contributor

Please investigate this request.

Contributor

rrelyea commented Jan 4, 2017

Please investigate this request.

@jviau

This comment has been minimized.

Show comment
Hide comment
@jviau

jviau Jan 4, 2017

@AArnott can you validate if my understanding of JoinableTaskFactory is correct? Is it necessary for NuGet to use the CPS JTF when interacting with CPS projects?

jviau commented Jan 4, 2017

@AArnott can you validate if my understanding of JoinableTaskFactory is correct? Is it necessary for NuGet to use the CPS JTF when interacting with CPS projects?

@AArnott

This comment has been minimized.

Show comment
Hide comment
@AArnott

AArnott Jan 7, 2017

Contributor

Yes. The CPS JoinableTaskFactory is aware of project locks and can mitigate what would otherwise be deadlocks between code that controls the main thread and code that holds project locks. So when using JoinableTaskFactory.Run or RunAsync, you should use the CPS one when the delegate you pass in may call into CPS code that needs project locks.

Contributor

AArnott commented Jan 7, 2017

Yes. The CPS JoinableTaskFactory is aware of project locks and can mitigate what would otherwise be deadlocks between code that controls the main thread and code that holds project locks. So when using JoinableTaskFactory.Run or RunAsync, you should use the CPS one when the delegate you pass in may call into CPS code that needs project locks.

@AArnott

This comment has been minimized.

Show comment
Hide comment

@rrelyea rrelyea modified the milestones: 4.0 RTM, 4.0 RC3 Jan 11, 2017

@rrelyea

This comment has been minimized.

Show comment
Hide comment
@rrelyea

rrelyea Jan 11, 2017

Contributor

Moving to RTM, as it is a bit risky for last day of RC3. Justin has fix.

Contributor

rrelyea commented Jan 11, 2017

Moving to RTM, as it is a bit risky for last day of RC3. Justin has fix.

emgarten added a commit to NuGet/NuGet.Client that referenced this issue Jan 12, 2017

Use IProjectThreadingService in NuGetUIThreadHelper
Update the NuGetUIThreadHelper to use IProjectThreadingService for VS 2017 and IThreadHandling for VS 2015. These JoinableTaskFactories are able to work with the CPS project system used by .NETCore projects and avoid hangs.

Fixes NuGet/Home#4185

emgarten added a commit to NuGet/NuGet.Client that referenced this issue Jan 12, 2017

Use IProjectThreadingService in NuGetUIThreadHelper
Update the NuGetUIThreadHelper to use IProjectThreadingService for VS 2017 and IThreadHandling for VS 2015. These JoinableTaskFactories are able to work with the CPS project system used by .NETCore projects and avoid hangs.

Fixes NuGet/Home#4185

emgarten added a commit to NuGet/NuGet.Client that referenced this issue Jan 13, 2017

Use IProjectThreadingService in NuGetUIThreadHelper
Update the NuGetUIThreadHelper to use IProjectThreadingService for VS 2017 and IThreadHandling for VS 2015. These JoinableTaskFactories are able to work with the CPS project system used by .NETCore projects and avoid hangs.

Fixes NuGet/Home#4185
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment