Skip to content
This repository was archived by the owner on Jun 21, 2023. It is now read-only.

Commit afb8d2d

Browse files
jcansdalejcansdale
authored
Merge pull request #1689 from github/fixes/1688-initialize-free-threaded
Make shared MEF component constructors free-threaded
2 parents fec41ac + 450343d commit afb8d2d

File tree

18 files changed

+255
-111
lines changed

18 files changed

+255
-111
lines changed

‎src/GitHub.App/Services/TeamExplorerContext.cs‎

Lines changed: 58 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,16 @@
33
using System.ComponentModel;
44
using System.ComponentModel.Composition;
55
using System.Reactive.Linq;
6+
using System.Threading.Tasks;
67
using GitHub.Models;
78
using GitHub.Logging;
89
using GitHub.Primitives;
10+
using GitHub.Extensions;
911
using Serilog;
1012
using EnvDTE;
13+
using Microsoft.VisualStudio.Shell;
14+
using Microsoft.VisualStudio.Threading;
15+
using Task = System.Threading.Tasks.Task;
1116

1217
namespace GitHub.Services
1318
{
@@ -28,7 +33,7 @@ public class TeamExplorerContext : ITeamExplorerContext
2833
{
2934
static ILogger log = LogManager.ForContext<TeamExplorerContext>();
3035

31-
readonly DTE dte;
36+
readonly AsyncLazy<DTE> dteAsync;
3237
readonly IVSGitExt gitExt;
3338
readonly IPullRequestService pullRequestService;
3439

@@ -41,30 +46,63 @@ public class TeamExplorerContext : ITeamExplorerContext
4146
Tuple<string, int> pullRequest;
4247

4348
ILocalRepositoryModel repositoryModel;
49+
JoinableTask refreshJoinableTask;
4450

4551
[ImportingConstructor]
52+
TeamExplorerContext(
53+
IVSGitExt gitExt,
54+
[Import(typeof(SVsServiceProvider))] IServiceProvider sp,
55+
IPullRequestService pullRequestService) : this(
56+
gitExt,
57+
new AsyncLazy<DTE>(async () =>
58+
{
59+
await ThreadHelper.JoinableTaskFactory.SwitchToMainThreadAsync();
60+
return (DTE)sp.GetService(typeof(DTE));
61+
}),
62+
pullRequestService,
63+
ThreadHelper.JoinableTaskContext)
64+
{
65+
}
66+
4667
public TeamExplorerContext(
47-
IGitHubServiceProvider serviceProvider,
4868
IVSGitExt gitExt,
49-
IPullRequestService pullRequestService)
69+
AsyncLazy<DTE> dteAsync,
70+
IPullRequestService pullRequestService,
71+
JoinableTaskContext joinableTaskContext)
5072
{
73+
JoinableTaskCollection = joinableTaskContext.CreateCollection();
74+
JoinableTaskCollection.DisplayName = nameof(TeamExplorerContext);
75+
JoinableTaskFactory = joinableTaskContext.CreateFactory(JoinableTaskCollection);
76+
5177
this.gitExt = gitExt;
78+
this.dteAsync = dteAsync;
5279
this.pullRequestService = pullRequestService;
5380

54-
// This is a standard service which should always be available.
55-
dte = serviceProvider.GetService<DTE>();
56-
57-
Refresh();
81+
StartRefresh();
5882
gitExt.ActiveRepositoriesChanged += Refresh;
5983
}
6084

61-
async void Refresh()
85+
void StartRefresh() => JoinableTaskFactory.RunAsync(QueueRefreshAsync).Task.Forget(log);
86+
void Refresh() => JoinableTaskFactory.Run(QueueRefreshAsync);
87+
88+
async Task QueueRefreshAsync()
89+
{
90+
if (refreshJoinableTask != null)
91+
{
92+
await refreshJoinableTask.JoinAsync(); // make sure StartRefresh has completed
93+
}
94+
95+
await (refreshJoinableTask = JoinableTaskFactory.RunAsync(RefreshAsync));
96+
}
97+
98+
async Task RefreshAsync()
6299
{
63100
try
64101
{
65-
var repo = gitExt.ActiveRepositories?.FirstOrDefault();
66-
var newSolutionPath = dte.Solution?.FullName;
102+
await TaskScheduler.Default; // switch to threadpool
67103

104+
var repo = gitExt.ActiveRepositories?.FirstOrDefault();
105+
string newSolutionPath = await GetSolutionPath();
68106
if (repo == null && newSolutionPath == solutionPath)
69107
{
70108
// Ignore when ActiveRepositories is empty and solution hasn't changed.
@@ -126,6 +164,13 @@ async void Refresh()
126164
}
127165
}
128166

167+
async Task<string> GetSolutionPath()
168+
{
169+
await JoinableTaskFactory.SwitchToMainThreadAsync();
170+
var dte = await dteAsync.GetValueAsync();
171+
return dte.Solution?.FullName;
172+
}
173+
129174
/// <summary>
130175
/// The active repository or null if not in a repository.
131176
/// </summary>
@@ -155,5 +200,8 @@ private set
155200
/// Fired when the current branch, head SHA or tracked SHA changes.
156201
/// </summary>
157202
public event EventHandler StatusChanged;
203+
204+
public JoinableTaskCollection JoinableTaskCollection { get; }
205+
JoinableTaskFactory JoinableTaskFactory { get; }
158206
}
159207
}

‎src/GitHub.App/ViewModels/GitHubPane/GitHubPaneViewModel.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -332,9 +332,9 @@ async Task CreateInitializeTask(IServiceProvider paneServiceProvider)
332332
teamExplorerContext.WhenAnyValue(x => x.ActiveRepository)
333333
.Skip(1)
334334
.ObserveOn(RxApp.MainThreadScheduler)
335-
.Subscribe(x => UpdateContent(x).Forget());
335+
.Subscribe(x => UpdateContent(x).Forget(log));
336336

337-
connectionManager.Connections.CollectionChanged += (_, __) => UpdateContent(LocalRepository).Forget();
337+
connectionManager.Connections.CollectionChanged += (_, __) => UpdateContent(LocalRepository).Forget(log);
338338

339339
var menuService = (IMenuCommandService)paneServiceProvider.GetService(typeof(IMenuCommandService));
340340
BindNavigatorCommand(menuService, PkgCmdIDList.pullRequestCommand, showPullRequests);

‎src/GitHub.Extensions/GitHub.Extensions.csproj‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,9 @@
5151
<HintPath>..\..\packages\Microsoft.VisualStudio.Shell.Immutable.10.0.10.0.30319\lib\net40\Microsoft.VisualStudio.Shell.Immutable.10.0.dll</HintPath>
5252
<Private>True</Private>
5353
</Reference>
54+
<Reference Include="Serilog, Version=2.0.0.0, Culture=neutral, PublicKeyToken=24c2f752a8e58a10, processorArchitecture=MSIL">
55+
<HintPath>..\..\packages\Serilog.2.5.0\lib\net46\Serilog.dll</HintPath>
56+
</Reference>
5457
<Reference Include="System" />
5558
<Reference Include="System.ComponentModel.Composition" />
5659
<Reference Include="System.Core" />

‎src/GitHub.Extensions/TaskExtensions.cs‎

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,14 @@
11
using System;
22
using System.Threading.Tasks;
3+
using GitHub.Logging;
4+
using Serilog;
35

46
namespace GitHub.Extensions
57
{
68
public static class TaskExtensions
79
{
10+
static readonly ILogger log = LogManager.ForContext(typeof(TaskExtensions));
11+
812
public static async Task<T> Catch<T>(this Task<T> source, Func<Exception, T> handler = null)
913
{
1014
Guard.ArgumentNotNull(source, nameof(source));
@@ -36,9 +40,37 @@ public static async Task Catch(this Task source, Action<Exception> handler = nul
3640
}
3741
}
3842

39-
[System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Usage", "CA1801:ReviewUnusedParameters", MessageId = "task")]
40-
public static void Forget(this Task task)
43+
/// <summary>
44+
/// Allow task to run and log any exceptions.
45+
/// </summary>
46+
/// <param name="task">The <see cref="Task"/> to log exceptions from.</param>
47+
/// <param name="errorMessage">An error message to log if the task throws.</param>
48+
public static void Forget(this Task task, string errorMessage = "")
49+
{
50+
task.ContinueWith(t =>
51+
{
52+
if (t.IsFaulted)
53+
{
54+
log.Error(t.Exception, errorMessage);
55+
}
56+
});
57+
}
58+
59+
/// <summary>
60+
/// Allow task to run and log any exceptions.
61+
/// </summary>
62+
/// <param name="task">The task to log exceptions from.</param>
63+
/// <param name="log">The logger to use.</param>
64+
/// <param name="errorMessage">The error message to log if the task throws.</param>
65+
public static void Forget(this Task task, ILogger log, string errorMessage = "")
4166
{
67+
task.ContinueWith(t =>
68+
{
69+
if (t.IsFaulted)
70+
{
71+
log.Error(t.Exception, errorMessage);
72+
}
73+
});
4274
}
4375
}
4476
}

‎src/GitHub.Extensions/packages.config‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
<?xml version="1.0" encoding="utf-8"?>
22
<packages>
33
<package id="Microsoft.VisualStudio.Shell.Immutable.10.0" version="10.0.30319" targetFramework="net461" />
4+
<package id="Serilog" version="2.5.0" targetFramework="net461" />
45
</packages>

‎src/GitHub.InlineReviews/Services/PullRequestSessionManager.cs‎

Lines changed: 18 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,8 @@
33
using System.ComponentModel.Composition;
44
using System.IO;
55
using System.Linq;
6-
using System.Reactive;
76
using System.Reactive.Disposables;
87
using System.Reactive.Linq;
9-
using System.Reactive.Threading.Tasks;
108
using System.Threading.Tasks;
119
using GitHub.Extensions;
1210
using GitHub.Factories;
@@ -61,11 +59,11 @@ public PullRequestSessionManager(
6159

6260
Observable.FromEventPattern(teamExplorerContext, nameof(teamExplorerContext.StatusChanged))
6361
.ObserveOn(RxApp.MainThreadScheduler)
64-
.Subscribe(_ => StatusChanged().Forget());
62+
.Subscribe(_ => StatusChanged().Forget(log));
6563

6664
teamExplorerContext.WhenAnyValue(x => x.ActiveRepository)
6765
.ObserveOn(RxApp.MainThreadScheduler)
68-
.Subscribe(x => RepoChanged(x).Forget());
66+
.Subscribe(x => RepoChanged(x).Forget(log));
6967
}
7068

7169
/// <inheritdoc/>
@@ -198,36 +196,29 @@ async Task RepoChanged(ILocalRepositoryModel localRepositoryModel)
198196

199197
async Task StatusChanged()
200198
{
201-
try
199+
var session = CurrentSession;
200+
201+
var pr = await service.GetPullRequestForCurrentBranch(repository).FirstOrDefaultAsync();
202+
if (pr != null)
202203
{
203-
var session = CurrentSession;
204+
var changePR =
205+
pr.Item1 != (session?.PullRequest.BaseRepositoryOwner) ||
206+
pr.Item2 != (session?.PullRequest.Number);
204207

205-
var pr = await service.GetPullRequestForCurrentBranch(repository).FirstOrDefaultAsync();
206-
if (pr != null)
208+
if (changePR)
207209
{
208-
var changePR =
209-
pr.Item1 != (session?.PullRequest.BaseRepositoryOwner) ||
210-
pr.Item2 != (session?.PullRequest.Number);
211-
212-
if (changePR)
213-
{
214-
var newSession = await GetSessionInternal(pr.Item1, repository.Name, pr.Item2);
215-
if (newSession != null) newSession.IsCheckedOut = true;
216-
session = newSession;
217-
}
210+
var newSession = await GetSessionInternal(pr.Item1, repository.Name, pr.Item2);
211+
if (newSession != null) newSession.IsCheckedOut = true;
212+
session = newSession;
218213
}
219-
else
220-
{
221-
session = null;
222-
}
223-
224-
CurrentSession = session;
225-
initialized.TrySetResult(null);
226214
}
227-
catch (Exception e)
215+
else
228216
{
229-
log.Error(e, "Error changing repository");
217+
session = null;
230218
}
219+
220+
CurrentSession = session;
221+
initialized.TrySetResult(null);
231222
}
232223

233224
async Task<PullRequestSession> GetSessionInternal(string owner, string name, int number)

‎src/GitHub.Logging/Logging/ILoggerExtensions.cs‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
using System;
21
using Serilog;
32

43
namespace GitHub.Logging

‎src/GitHub.TeamFoundation.14/Base/TeamExplorerServiceHolder.cs‎

Lines changed: 35 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2,46 +2,60 @@
22
using System.Collections.Generic;
33
using System.ComponentModel.Composition;
44
using System.Linq;
5-
using System.Threading;
65
using GitHub.Extensions;
6+
using GitHub.Logging;
77
using GitHub.Models;
88
using GitHub.Services;
9+
using Serilog;
910
using Microsoft.TeamFoundation.Controls;
11+
using Microsoft.VisualStudio.Shell;
12+
using Microsoft.VisualStudio.Threading;
13+
using System.Windows;
1014

1115
namespace GitHub.VisualStudio.Base
1216
{
1317
[Export(typeof(ITeamExplorerServiceHolder))]
1418
[PartCreationPolicy(CreationPolicy.Shared)]
1519
public class TeamExplorerServiceHolder : ITeamExplorerServiceHolder
1620
{
21+
static readonly ILogger log = LogManager.ForContext<TeamExplorerServiceHolder>();
22+
1723
readonly Dictionary<object, Action<ILocalRepositoryModel>> activeRepoHandlers = new Dictionary<object, Action<ILocalRepositoryModel>>();
1824
ILocalRepositoryModel activeRepo;
1925
bool activeRepoNotified = false;
2026

2127
IServiceProvider serviceProvider;
2228
readonly IVSGitExt gitService;
2329

24-
// ActiveRepositories PropertyChanged event comes in on a non-main thread
25-
readonly SynchronizationContext syncContext;
26-
2730
/// <summary>
2831
/// This class relies on IVSGitExt that provides information when VS switches repositories.
2932
/// </summary>
3033
/// <param name="gitService">Used for monitoring the active repository.</param>
3134
[ImportingConstructor]
32-
public TeamExplorerServiceHolder(IVSGitExt gitService)
35+
TeamExplorerServiceHolder(IVSGitExt gitService) : this(gitService, ThreadHelper.JoinableTaskContext)
36+
{
37+
}
38+
39+
/// <summary>
40+
/// This constructor can be used for unit testing.
41+
/// </summary>
42+
/// <param name="gitService">Used for monitoring the active repository.</param>
43+
/// <param name="joinableTaskFactory">Used for switching to the Main thread.</param>
44+
public TeamExplorerServiceHolder(IVSGitExt gitService, JoinableTaskContext joinableTaskContext)
3345
{
34-
this.gitService = gitService;
35-
syncContext = SynchronizationContext.Current;
46+
JoinableTaskCollection = joinableTaskContext.CreateCollection();
47+
JoinableTaskCollection.DisplayName = nameof(TeamExplorerServiceHolder);
48+
JoinableTaskFactory = joinableTaskContext.CreateFactory(JoinableTaskCollection);
3649

37-
UpdateActiveRepo();
50+
// This might be null in Blend or SafeMode
3851
if (gitService != null)
3952
{
53+
this.gitService = gitService;
54+
UpdateActiveRepo();
4055
gitService.ActiveRepositoriesChanged += UpdateActiveRepo;
4156
}
4257
}
4358

44-
4559
// set by the sections when they get initialized
4660
public IServiceProvider ServiceProvider
4761
{
@@ -138,12 +152,17 @@ void NotifyActiveRepo()
138152

139153
void UpdateActiveRepo()
140154
{
141-
// NOTE: gitService might be null in Blend or Safe Mode
142-
var repo = gitService?.ActiveRepositories.FirstOrDefault();
155+
var repo = gitService.ActiveRepositories.FirstOrDefault();
143156

144157
if (!Equals(repo, ActiveRepo))
145-
// so annoying that this is on the wrong thread
146-
syncContext.Post(r => ActiveRepo = r as ILocalRepositoryModel, repo);
158+
{
159+
// Fire property change events on Main thread
160+
JoinableTaskFactory.RunAsync(async () =>
161+
{
162+
await JoinableTaskFactory.SwitchToMainThreadAsync();
163+
ActiveRepo = repo;
164+
}).Task.Forget(log);
165+
}
147166
}
148167

149168
void ActiveRepoPropertyChanged(object sender, System.ComponentModel.PropertyChangedEventArgs e)
@@ -171,5 +190,8 @@ ITeamExplorerPage PageService
171190
{
172191
get { return ServiceProvider.GetServiceSafe<ITeamExplorerPage>(); }
173192
}
193+
194+
public JoinableTaskCollection JoinableTaskCollection { get; }
195+
JoinableTaskFactory JoinableTaskFactory { get; }
174196
}
175197
}

0 commit comments

Comments
 (0)