Restart the project using the same profile it get launched instead of the current active profile #9656
Conversation
…ent active profile
...Studio.ProjectSystem.Managed.VS/ProjectSystem/VS/HotReload/ProjectHotReloadSessionManager.cs
Outdated
Show resolved
Hide resolved
...Studio.ProjectSystem.Managed.VS/ProjectSystem/VS/HotReload/ProjectHotReloadSessionManager.cs
Outdated
Show resolved
Hide resolved
...Studio.ProjectSystem.Managed.VS/ProjectSystem/VS/HotReload/ProjectHotReloadSessionManager.cs
Outdated
Show resolved
Hide resolved
...Studio.ProjectSystem.Managed.VS/ProjectSystem/VS/HotReload/ProjectHotReloadSessionManager.cs
Outdated
Show resolved
Hide resolved
...Studio.ProjectSystem.Managed.VS/ProjectSystem/VS/HotReload/ProjectHotReloadSessionManager.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/VSResources.resx
Outdated
Show resolved
Hide resolved
...lStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/Debug/LaunchProfilesDebugLaunchProvider.cs
Outdated
Show resolved
Hide resolved
...lStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/Debug/LaunchProfilesDebugLaunchProvider.cs
Outdated
Show resolved
Hide resolved
....VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/HotReload/ProjectHotReloadSession.cs
Outdated
Show resolved
Hide resolved
....VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/HotReload/ProjectHotReloadSession.cs
Outdated
Show resolved
Hide resolved
| while (Interlocked.CompareExchange(ref _buildLeft, 0, 0) > 0) | ||
| { | ||
| // Wait for the build to complete | ||
| await Task.Yield(); |
There was a problem hiding this comment.
This code is doing the following every 100ms or so:
- Readding itself to the back of the message pump via
Task.Yield() - Yielding and waiting for the timer to fire on the thread pool
Task.Delay - Then returns to its original context (UI thread), so gets implicitly gets readded back to the message pump, then starts again at 1)
I suspect this might also cause Idle to never fire, which will never update command states.
There is likely a more efficient way to do this via TaskCompletionSource, but what problem is this code solving?
There was a problem hiding this comment.
What WaitForSolutionBuildCompletedAsync trying to do is blocking until the project build to be completed from SolutionBuildManager by subscribing its UpdateSolution_Start and periodically check if UpdateSolution_Done or UpdateSolution_Cancel get called.
There's a discussion on using TaskCompletionSource<bool> and in WaitForSolutionBuildCompletedAsync , simply waiting for its task to be completed.(see commets), The concern to it though is TaskCompletionSource<bool> is a blackbox to JTF and might lead to hang. If using TaskCompletionSource<bool> is more efficient, how to use it correctly.
There was a problem hiding this comment.
I think I understand why you wrote this; you are trying to be "JTF-friendly" if someone blocks on it from the UI thread via JTF.Run. But even if this was efficient, it would never work as the bit you are waiting to be set (_buildLeft) can never be set because the solution build manager would never get the UI thread because you are stuck in the loop waiting for it.
If you absolutely must return a task to the caller, use TaskCompletionSource like you previously had but be aware that absolutely no one calling BuildProjectAsync can ever, ever block the UI thread waiting on it, because if they did:
- It would hang
- Even if it didn't hang, it would hang the UI thread while build was running.
I would at minimum inside BuildProjectAsync put something like:
if (ThreadHelper.JoinableTaskContext.IsMainThreadBlocked())
throw new InvalidOperationException("This task cannot be blocked on by the UI thread.")This would miss the case where the task is joined by the UI thread after it started, but I need to think about patterns to avoid that.
There was a problem hiding this comment.
@LittleLittleCloud, TaskCompletionSource is likely the solution here, but with the JTF protection mentioned in the article I linked to in an earlier comment.
EDIT @davkean's comment didn't appear when I posted this. I'm less clear on his solution, but it seems reasonable.
There was a problem hiding this comment.
@drewnoakes Mentioned this pattern but it won't prevent the hang in TaskCompletionSource usage because you will hit this case it calls out:
This works fine when the class itself fully controls the work to complete the TaskCompletionSource. When other classes also do work (independently of work started within SomeClass), the placement and access to the JoinableTaskFactory that is associated with the JoinableTaskCollection may need to be elevated so that other classes can access it as well so that all the work required to complete the TaskCompletionSource will be tracked.
Even then, we wouldn't want anyone to block on it via the UI thread because we want the UI thread to process user messages while the product is building.
There was a problem hiding this comment.
But even if this was efficient, it would never work as the bit you are waiting to be set (_buildLeft) can never be set because the solution build manager would never get the UI thread because you are stuck in the loop waiting for it.
Can you elaborate more on this one, the WaitForSolutionBuildCompletedAsync explicitly yield the task in every loop, would that prevent hanging in some way?
There was a problem hiding this comment.
If JTF.Run was wrapped around this async method, it wouldn't pump anything on the UI thread except the Task.Yield (it would actually run it immediately) and the implicit UI thread switch after the Task.Delay. The event handler would never run because JTF doesn't recognize that it needs to run it for you to break out of the loop.
There was a problem hiding this comment.
Thanks @davkean
I update the code to track setting TaskCompletionSource in JoinableTaskCollection and waiting their join when waiting on TaskCompletionSource.Task. Could you review and see if it's done correctly
There was a problem hiding this comment.
@drewnoakes @LittleLittleCloud This doesn't prevent the hang because of this problem that I called out:
@drewnoakes Mentioned this pattern but it won't prevent the hang in TaskCompletionSource usage because you will hit this case it calls out:
This works fine when the class itself fully controls the work to complete the TaskCompletionSource. When other classes also do work (independently of work started within SomeClass), the placement and access to the JoinableTaskFactory that is associated with the JoinableTaskCollection may need to be elevated so that other classes can access it as well so that all the work required to complete the TaskCompletionSource will be tracked.
Even then, we wouldn't want anyone to block on it via the UI thread because we want the UI thread to process user messages while the product is building.
If someone calls JTF.Run on the UI thread around this task, you will not pump any message that calls UpdateSolution_Done and UpdateSolution_Cancel because they them E864 selves need the UI thread and have no JTF relationship to your code.
I will repeat, avoid trying to prevent the hang - we would never want anyone to block the UI thread on this task as it will hang Visual Studio while building.
There was a problem hiding this comment.
Finally get some time to get back to this PR
I revert the changes of attempting to be JTF-friendly and instead, adding a check to raise exception when BuildProjectAsync is called on main thread
...VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/Debug/IInternalDebugLaunchProvider.cs
Show resolved
Hide resolved
...VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/Debug/IInternalDebugLaunchProvider.cs
Outdated
Show resolved
Hide resolved
....VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/HotReload/ProjectHotReloadSession.cs
Outdated
Show resolved
Hide resolved
....VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/HotReload/ProjectHotReloadSession.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
LGTM with some minor comments. Would like @davkean to review the threading concerns he raised earlier.
Microsoft Reviewers: Open in CodeFlow
Resolve #9657 by taking record of the previous profile and debug option in
ProjectHotReloadSessionand uses these value to restart project