github · jcansdale · Jun 22, 2018 · May 18, 2018 · May 18, 2018 · May 18, 2018
diff --git a/src/GitHub.App/Services/TeamExplorerContext.cs b/src/GitHub.App/Services/TeamExplorerContext.cs
@@ -3,11 +3,16 @@
 using System.ComponentModel;
 using System.ComponentModel.Composition;
 using System.Reactive.Linq;
+using System.Threading.Tasks;
 using GitHub.Models;
 using GitHub.Logging;
 using GitHub.Primitives;
+using GitHub.Extensions;
 using Serilog;
 using EnvDTE;
+using Microsoft.VisualStudio.Shell;
+using Microsoft.VisualStudio.Threading;
+using Task = System.Threading.Tasks.Task;
 
 namespace GitHub.Services
 {
@@ -28,7 +33,7 @@ public class TeamExplorerContext : ITeamExplorerContext
     {
         static ILogger log = LogManager.ForContext<TeamExplorerContext>();
 
-        readonly DTE dte;
+        readonly AsyncLazy<DTE> dteAsync;
         readonly IVSGitExt gitExt;
         readonly IPullRequestService pullRequestService;
 
@@ -41,30 +46,63 @@ public class TeamExplorerContext : ITeamExplorerContext
         Tuple<string, int> pullRequest;
 
         ILocalRepositoryModel repositoryModel;
+        JoinableTask refreshJoinableTask;
 
         [ImportingConstructor]
+        TeamExplorerContext(
+            IVSGitExt gitExt,
+            [Import(typeof(SVsServiceProvider))] IServiceProvider sp,
+            IPullRequestService pullRequestService) : this(
+                gitExt,
+                new AsyncLazy<DTE>(async () =>
+                {
+                    await ThreadHelper.JoinableTaskFactory.SwitchToMainThreadAsync();
+                    return (DTE)sp.GetService(typeof(DTE));
+                }),
+                pullRequestService,
+                ThreadHelper.JoinableTaskContext)
+        {
+        }
+
         public TeamExplorerContext(
-            IGitHubServiceProvider serviceProvider,
             IVSGitExt gitExt,
-            IPullRequestService pullRequestService)
+            AsyncLazy<DTE> dteAsync,
+            IPullRequestService pullRequestService,
+            JoinableTaskContext joinableTaskContext)
         {
+            JoinableTaskCollection = joinableTaskContext.CreateCollection();
+            JoinableTaskCollection.DisplayName = nameof(TeamExplorerContext);
+            JoinableTaskFactory = joinableTaskContext.CreateFactory(JoinableTaskCollection);
+
             this.gitExt = gitExt;
+            this.dteAsync = dteAsync;
             this.pullRequestService = pullRequestService;
 
-            // This is a standard service which should always be available.
-            dte = serviceProvider.GetService<DTE>();
-
-            Refresh();
+            StartRefresh();
             gitExt.ActiveRepositoriesChanged += Refresh;
         }
 
-        async void Refresh()
+        void StartRefresh() => JoinableTaskFactory.RunAsync(QueueRefreshAsync).Task.Forget(log);
+        void Refresh() => JoinableTaskFactory.Run(QueueRefreshAsync);
+
+        async Task QueueRefreshAsync()
+        {
+            if (refreshJoinableTask != null)
+            {
+                await refreshJoinableTask.JoinAsync(); // make sure StartRefresh has completed
+            }
+
+            await (refreshJoinableTask = JoinableTaskFactory.RunAsync(RefreshAsync));
+        }
+
+        async Task RefreshAsync()
         {
             try
             {
-                var repo = gitExt.ActiveRepositories?.FirstOrDefault();
-                var newSolutionPath = dte.Solution?.FullName;
+                await TaskScheduler.Default; // switch to threadpool
 
+                var repo = gitExt.ActiveRepositories?.FirstOrDefault();
+                string newSolutionPath = await GetSolutionPath();
                 if (repo == null && newSolutionPath == solutionPath)
                 {
                     // Ignore when ActiveRepositories is empty and solution hasn't changed.
@@ -126,6 +164,13 @@ async void Refresh()
             }
         }
 
+        async Task<string> GetSolutionPath()
+        {
+            await JoinableTaskFactory.SwitchToMainThreadAsync();
+            var dte = await dteAsync.GetValueAsync();
+            return dte.Solution?.FullName;
+        }
+
         /// <summary>
         /// The active repository or null if not in a repository.
         /// </summary>
@@ -155,5 +200,8 @@ private set
         /// Fired when the current branch, head SHA or tracked SHA changes.
         /// </summary>
         public event EventHandler StatusChanged;
+
+        public JoinableTaskCollection JoinableTaskCollection { get; }
+        JoinableTaskFactory JoinableTaskFactory { get; }
     }
 }
diff --git a/src/GitHub.App/ViewModels/GitHubPane/GitHubPaneViewModel.cs b/src/GitHub.App/ViewModels/GitHubPane/GitHubPaneViewModel.cs
@@ -332,9 +332,9 @@ async Task CreateInitializeTask(IServiceProvider paneServiceProvider)
             teamExplorerContext.WhenAnyValue(x => x.ActiveRepository)
                .Skip(1)
                .ObserveOn(RxApp.MainThreadScheduler)
-               .Subscribe(x => UpdateContent(x).Forget());
+               .Subscribe(x => UpdateContent(x).Forget(log));
 
-            connectionManager.Connections.CollectionChanged += (_, __) => UpdateContent(LocalRepository).Forget();
+            connectionManager.Connections.CollectionChanged += (_, __) => UpdateContent(LocalRepository).Forget(log);
 
             var menuService = (IMenuCommandService)paneServiceProvider.GetService(typeof(IMenuCommandService));
             BindNavigatorCommand(menuService, PkgCmdIDList.pullRequestCommand, showPullRequests);

diff --git a/src/GitHub.Extensions/GitHub.Extensions.csproj b/src/GitHub.Extensions/GitHub.Extensions.csproj
@@ -51,6 +51,9 @@
       <HintPath>..\..\packages\Microsoft.VisualStudio.Shell.Immutable.10.0.10.0.30319\lib\net40\Microsoft.VisualStudio.Shell.Immutable.10.0.dll</HintPath>
       <Private>True</Private>
     </Reference>
+    <Reference Include="Serilog, Version=2.0.0.0, Culture=neutral, PublicKeyToken=24c2f752a8e58a10, processorArchitecture=MSIL">
+      <HintPath>..\..\packages\Serilog.2.5.0\lib\net46\Serilog.dll</HintPath>
+    </Reference>
     <Reference Include="System" />
     <Reference Include="System.ComponentModel.Composition" />
     <Reference Include="System.Core" />

diff --git a/src/GitHub.Extensions/TaskExtensions.cs b/src/GitHub.Extensions/TaskExtensions.cs
@@ -1,10 +1,14 @@
 using System;
 using System.Threading.Tasks;
+using GitHub.Logging;
+using Serilog;
 
 namespace GitHub.Extensions
 {
     public static class TaskExtensions
     {
+        static readonly ILogger log = LogManager.ForContext(typeof(TaskExtensions));
+
         public static async Task<T> Catch<T>(this Task<T> source, Func<Exception, T> handler = null)
         {
             Guard.ArgumentNotNull(source, nameof(source));
@@ -36,9 +40,37 @@ public static async Task Catch(this Task source, Action<Exception> handler = nul
             }
         }
 
-        [System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Usage", "CA1801:ReviewUnusedParameters", MessageId = "task")]
-        public static void Forget(this Task task)
+        /// <summary>
+        /// Allow task to run and log any exceptions.
+        /// </summary>
+        /// <param name="task">The <see cref="Task"/> to log exceptions from.</param>
+        /// <param name="errorMessage">An error message to log if the task throws.</param>
+        public static void Forget(this Task task, string errorMessage = "")
+        {
+            task.ContinueWith(t =>
+            {
+                if (t.IsFaulted)
+                {
+                    log.Error(t.Exception, errorMessage);
+                }
+            });
+        }
+
+        /// <summary>
+        /// Allow task to run and log any exceptions.
+        /// </summary>
+        /// <param name="task">The task to log exceptions from.</param>
+        /// <param name="log">The logger to use.</param>
+        /// <param name="errorMessage">The error message to log if the task throws.</param>
+        public static void Forget(this Task task, ILogger log, string errorMessage = "")
         {
+            task.ContinueWith(t =>
+            {
+                if (t.IsFaulted)
+                {
+                    log.Error(t.Exception, errorMessage);
+                }
+            });
         }
     }
 }
diff --git a/src/GitHub.Extensions/packages.config b/src/GitHub.Extensions/packages.config
@@ -1,4 +1,5 @@
 <?xml version="1.0" encoding="utf-8"?>
 <packages>
   <package id="Microsoft.VisualStudio.Shell.Immutable.10.0" version="10.0.30319" targetFramework="net461" />
+  <package id="Serilog" version="2.5.0" targetFramework="net461" />
 </packages>
diff --git a/src/GitHub.InlineReviews/Services/PullRequestSessionManager.cs b/src/GitHub.InlineReviews/Services/PullRequestSessionManager.cs
@@ -3,10 +3,8 @@
 using System.ComponentModel.Composition;
 using System.IO;
 using System.Linq;
-using System.Reactive;
 using System.Reactive.Disposables;
 using System.Reactive.Linq;
-using System.Reactive.Threading.Tasks;
 using System.Threading.Tasks;
 using GitHub.Extensions;
 using GitHub.Factories;
@@ -61,11 +59,11 @@ public PullRequestSessionManager(
 
             Observable.FromEventPattern(teamExplorerContext, nameof(teamExplorerContext.StatusChanged))
                 .ObserveOn(RxApp.MainThreadScheduler)
-                .Subscribe(_ => StatusChanged().Forget());
+                .Subscribe(_ => StatusChanged().Forget(log));
 
             teamExplorerContext.WhenAnyValue(x => x.ActiveRepository)
                 .ObserveOn(RxApp.MainThreadScheduler)
-                .Subscribe(x => RepoChanged(x).Forget());
+                .Subscribe(x => RepoChanged(x).Forget(log));
         }
 
         /// <inheritdoc/>
@@ -198,36 +196,29 @@ async Task RepoChanged(ILocalRepositoryModel localRepositoryModel)
 
         async Task StatusChanged()
         {
-            try
+            var session = CurrentSession;
+
+            var pr = await service.GetPullRequestForCurrentBranch(repository).FirstOrDefaultAsync();
+            if (pr != null)
             {
-                var session = CurrentSession;
+                var changePR =
+                    pr.Item1 != (session?.PullRequest.BaseRepositoryOwner) ||
+                    pr.Item2 != (session?.PullRequest.Number);
 
-                var pr = await service.GetPullRequestForCurrentBranch(repository).FirstOrDefaultAsync();
-                if (pr != null)
+                if (changePR)
                 {
-                    var changePR =
-                        pr.Item1 != (session?.PullRequest.BaseRepositoryOwner) ||
-                        pr.Item2 != (session?.PullRequest.Number);
-
-                    if (changePR)
-                    {
-                        var newSession = await GetSessionInternal(pr.Item1, repository.Name, pr.Item2);
-                        if (newSession != null) newSession.IsCheckedOut = true;
-                        session = newSession;
-                    }
+                    var newSession = await GetSessionInternal(pr.Item1, repository.Name, pr.Item2);
+                    if (newSession != null) newSession.IsCheckedOut = true;
+                    session = newSession;
                 }
-                else
-                {
-                    session = null;
-                }
-
-                CurrentSession = session;
-                initialized.TrySetResult(null);
             }
-            catch (Exception e)
+            else
             {
-                log.Error(e, "Error changing repository");
+                session = null;
             }
+
+            CurrentSession = session;
+            initialized.TrySetResult(null);
         }
 
         async Task<PullRequestSession> GetSessionInternal(string owner, string name, int number)

diff --git a/src/GitHub.Logging/Logging/ILoggerExtensions.cs b/src/GitHub.Logging/Logging/ILoggerExtensions.cs
@@ -1,4 +1,3 @@
-using System;
 using Serilog;
 
 namespace GitHub.Logging

diff --git a/src/GitHub.TeamFoundation.14/Base/TeamExplorerServiceHolder.cs b/src/GitHub.TeamFoundation.14/Base/TeamExplorerServiceHolder.cs
@@ -2,46 +2,60 @@
 using System.Collections.Generic;
 using System.ComponentModel.Composition;
 using System.Linq;
-using System.Threading;
 using GitHub.Extensions;
+using GitHub.Logging;
 using GitHub.Models;
 using GitHub.Services;
+using Serilog;
 using Microsoft.TeamFoundation.Controls;
+using Microsoft.VisualStudio.Shell;
+using Microsoft.VisualStudio.Threading;
+using System.Windows;
 
 namespace GitHub.VisualStudio.Base
 {
     [Export(typeof(ITeamExplorerServiceHolder))]
     [PartCreationPolicy(CreationPolicy.Shared)]
     public class TeamExplorerServiceHolder : ITeamExplorerServiceHolder
     {
+        static readonly ILogger log = LogManager.ForContext<TeamExplorerServiceHolder>();
+
         readonly Dictionary<object, Action<ILocalRepositoryModel>> activeRepoHandlers = new Dictionary<object, Action<ILocalRepositoryModel>>();
         ILocalRepositoryModel activeRepo;
         bool activeRepoNotified = false;
 
         IServiceProvider serviceProvider;
         readonly IVSGitExt gitService;
 
-        // ActiveRepositories PropertyChanged event comes in on a non-main thread
-        readonly SynchronizationContext syncContext;
-
         /// <summary>
         /// This class relies on IVSGitExt that provides information when VS switches repositories.
         /// </summary>
         /// <param name="gitService">Used for monitoring the active repository.</param>
         [ImportingConstructor]
-        public TeamExplorerServiceHolder(IVSGitExt gitService)
+        TeamExplorerServiceHolder(IVSGitExt gitService) : this(gitService, ThreadHelper.JoinableTaskContext)
+        {
+        }
+
+        /// <summary>
+        /// This constructor can be used for unit testing.
+        /// </summary>
+        /// <param name="gitService">Used for monitoring the active repository.</param>
+        /// <param name="joinableTaskFactory">Used for switching to the Main thread.</param>
+        public TeamExplorerServiceHolder(IVSGitExt gitService, JoinableTaskContext joinableTaskContext)
         {
-            this.gitService = gitService;
-            syncContext = SynchronizationContext.Current;
+            JoinableTaskCollection = joinableTaskContext.CreateCollection();
+            JoinableTaskCollection.DisplayName = nameof(TeamExplorerServiceHolder);
+            JoinableTaskFactory = joinableTaskContext.CreateFactory(JoinableTaskCollection);
 
-            UpdateActiveRepo();
+            // This might be null in Blend or SafeMode
             if (gitService != null)
             {
+                this.gitService = gitService;
+                UpdateActiveRepo();
                 gitService.ActiveRepositoriesChanged += UpdateActiveRepo;
             }
         }
 
-
         // set by the sections when they get initialized
         public IServiceProvider ServiceProvider
         {
@@ -138,12 +152,17 @@ void NotifyActiveRepo()
 
         void UpdateActiveRepo()
         {
-            // NOTE: gitService might be null in Blend or Safe Mode
-            var repo = gitService?.ActiveRepositories.FirstOrDefault();
+            var repo = gitService.ActiveRepositories.FirstOrDefault();
 
             if (!Equals(repo, ActiveRepo))
-                // so annoying that this is on the wrong thread
-                syncContext.Post(r => ActiveRepo = r as ILocalRepositoryModel, repo);
+            {
+                // Fire property change events on Main thread
+                JoinableTaskFactory.RunAsync(async () =>
+                {
+                    await JoinableTaskFactory.SwitchToMainThreadAsync();
+                    ActiveRepo = repo;
+                }).Task.Forget(log);
+            }
         }
 
         void ActiveRepoPropertyChanged(object sender, System.ComponentModel.PropertyChangedEventArgs e)
@@ -171,5 +190,8 @@ ITeamExplorerPage PageService
         {
             get { return ServiceProvider.GetServiceSafe<ITeamExplorerPage>(); }
         }
+
+        public JoinableTaskCollection JoinableTaskCollection { get; }
+        JoinableTaskFactory JoinableTaskFactory { get; }
     }
 }