Skip to content

Commit d814f1f

Browse files
committed
Ensure all app resources are disposed on exit
Refactor the main `Application` class to be disposed on application exit. All components that have `IDisposable` object references have now been given a chance to dispose of them.
1 parent c016a78 commit d814f1f

File tree

10 files changed

+146
-43
lines changed

10 files changed

+146
-43
lines changed

src/Microsoft.AzureRepos/AzureDevOpsRestApi.cs

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212

1313
namespace Microsoft.AzureRepos
1414
{
15-
public interface IAzureDevOpsRestApi
15+
public interface IAzureDevOpsRestApi : IDisposable
1616
{
1717
Task<string> GetAuthorityAsync(Uri organizationUri);
1818
Task<string> CreatePersonalAccessTokenAsync(Uri organizationUri, string accessToken, IEnumerable<string> scopes);
@@ -289,7 +289,7 @@ private static HttpRequestMessage CreateRequestMessage(HttpMethod method, Uri ur
289289
request.Content = content;
290290
}
291291

292-
if (!(bearerToken is null))
292+
if (!string.IsNullOrWhiteSpace(bearerToken))
293293
{
294294
request.Headers.Authorization = new AuthenticationHeaderValue(Constants.Http.WwwAuthenticateBearerScheme, bearerToken);
295295
}
@@ -298,5 +298,14 @@ private static HttpRequestMessage CreateRequestMessage(HttpMethod method, Uri ur
298298
}
299299

300300
#endregion
301+
302+
#region IDisposable
303+
304+
public void Dispose()
305+
{
306+
_httpClient?.Dispose();
307+
}
308+
309+
#endregion
301310
}
302311
}

src/Microsoft.AzureRepos/AzureReposHostProvider.cs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,16 @@ public override async Task<GitCredential> CreateCredentialAsync(InputArguments i
8585
return new GitCredential(Constants.PersonalAccessTokenUserName, pat);
8686
}
8787

88+
protected override void Dispose(bool disposing)
89+
{
90+
if (disposing)
91+
{
92+
_azDevOps.Dispose();
93+
}
94+
95+
base.Dispose(disposing);
96+
}
97+
8898
#endregion
8999
}
90100
}

src/Microsoft.Git.CredentialManager/Authentication/WindowsIntegratedAuthentication.cs

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77

88
namespace Microsoft.Git.CredentialManager.Authentication
99
{
10-
public interface IWindowsIntegratedAuthentication
10+
public interface IWindowsIntegratedAuthentication : IDisposable
1111
{
1212
Task<bool> GetIsSupportedAsync(Uri uri);
1313
}
@@ -58,5 +58,14 @@ public async Task<bool> GetIsSupportedAsync(Uri uri)
5858

5959
private HttpClient _httpClient;
6060
private HttpClient HttpClient => _httpClient ?? (_httpClient = _httpFactory.CreateClient());
61+
62+
#region IDisposable
63+
64+
public void Dispose()
65+
{
66+
_httpClient?.Dispose();
67+
}
68+
69+
#endregion
6170
}
6271
}

src/Microsoft.Git.CredentialManager/GenericHostProvider.cs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,12 @@ public override async Task<GitCredential> CreateCredentialAsync(InputArguments i
7171
return _basicAuth.GetCredentials(uri.AbsoluteUri, uri.UserInfo);
7272
}
7373

74+
protected override void Dispose(bool disposing)
75+
{
76+
_winAuth.Dispose();
77+
base.Dispose(disposing);
78+
}
79+
7480
#endregion
7581

7682
#region Helpers

src/Microsoft.Git.CredentialManager/HostProvider.cs

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,15 @@
11
// Copyright (c) Microsoft Corporation. All rights reserved.
22
// Licensed under the MIT license.
3+
4+
using System;
35
using System.Threading.Tasks;
46

57
namespace Microsoft.Git.CredentialManager
68
{
79
/// <summary>
810
/// Represents a particular Git hosting service and provides for the creation of credentials to access the remote.
911
/// </summary>
10-
public interface IHostProvider
12+
public interface IHostProvider : IDisposable
1113
{
1214
/// <summary>
1315
/// Name of the hosting provider.
@@ -60,5 +62,22 @@ protected HostProvider(ICommandContext context)
6062
public abstract string GetCredentialKey(InputArguments input);
6163

6264
public abstract Task<GitCredential> CreateCredentialAsync(InputArguments input);
65+
66+
/// <summary>
67+
/// Called when the application is being terminated. Clean up and release any resources.
68+
/// </summary>
69+
/// <param name="disposing">True if the instance is being disposed, false if being finalized.</param>
70+
protected virtual void Dispose(bool disposing) { }
71+
72+
public void Dispose()
73+
{
74+
Dispose(true);
75+
GC.SuppressFinalize(this);
76+
}
77+
78+
~HostProvider()
79+
{
80+
Dispose(false);
81+
}
6382
}
6483
}

src/Microsoft.Git.CredentialManager/HostProviderRegistry.cs

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,16 @@ namespace Microsoft.Git.CredentialManager
1010
/// Represents a collection of <see cref="IHostProvider"/>s which are selected based on Git credential query
1111
/// <see cref="InputArguments"/>.
1212
/// </summary>
13-
public interface IHostProviderRegistry
13+
/// <remarks>
14+
/// All registered <see cref="IHostProvider"/>s will be disposed when this <see cref="IHostProviderRegistry"/> is disposed.
15+
/// </remarks>
16+
public interface IHostProviderRegistry : IDisposable
1417
{
1518
/// <summary>
1619
/// Add the given <see cref="IHostProvider"/>(s) to this registry.
1720
/// </summary>
1821
/// <param name="hostProviders">A collection of providers to register.</param>
22+
/// <remarks>Providers will be disposed of when this registry instance is disposed itself.</remarks>
1923
void Register(params IHostProvider[] hostProviders);
2024

2125
/// <summary>
@@ -56,5 +60,14 @@ public IHostProvider GetProvider(InputArguments input)
5660

5761
return provider;
5862
}
63+
64+
public void Dispose()
65+
{
66+
// Dispose of all registered providers to give them a chance to clean up and release any resources
67+
foreach (IHostProvider provider in _hostProviders)
68+
{
69+
provider.Dispose();
70+
}
71+
}
5972
}
6073
}

src/git-credential-manager/Application.cs

Lines changed: 59 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -12,105 +12,116 @@
1212

1313
namespace Microsoft.Git.CredentialManager
1414
{
15-
public static class Application
15+
public class Application : IDisposable
1616
{
17-
private static readonly IHostProviderRegistry HostProviderRegistry = new HostProviderRegistry();
17+
private readonly ICommandContext _context;
18+
private readonly IHostProviderRegistry _providerRegistry = new HostProviderRegistry();
1819

19-
private static readonly ICollection<CommandBase> Commands = new CommandBase[]
20-
{
21-
new EraseCommand(HostProviderRegistry),
22-
new GetCommand(HostProviderRegistry),
23-
new StoreCommand(HostProviderRegistry),
24-
new VersionCommand(),
25-
new HelpCommand(),
26-
};
27-
28-
public static async Task<int> RunAsync(string[] args)
20+
private TextWriter _traceFileWriter;
21+
22+
public Application(ICommandContext context)
2923
{
30-
var context = new CommandContext();
24+
EnsureArgument.NotNull(context, nameof(context));
25+
26+
_context = context;
27+
}
3128

29+
public async Task<int> RunAsync(string[] args)
30+
{
3231
// Launch debugger
33-
if (context.IsEnvironmentVariableTruthy(Constants.EnvironmentVariables.GcmDebug, false))
32+
if (_context.IsEnvironmentVariableTruthy(Constants.EnvironmentVariables.GcmDebug, false))
3433
{
35-
context.StdError.WriteLine("Waiting for debugger to be attached...");
34+
_context.StdError.WriteLine("Waiting for debugger to be attached...");
3635
PlatformUtils.WaitForDebuggerAttached();
3736

3837
// Now the debugger is attached, break!
3938
Debugger.Break();
4039
}
4140

4241
// Enable tracing
43-
if (context.TryGetEnvironmentVariable(Constants.EnvironmentVariables.GcmTrace, out string traceEnvar))
42+
if (_context.TryGetEnvironmentVariable(Constants.EnvironmentVariables.GcmTrace, out string traceEnvar))
4443
{
4544
if (traceEnvar.IsTruthy()) // Trace to stderr
4645
{
47-
context.Trace.AddListener(context.StdError);
46+
_context.Trace.AddListener(_context.StdError);
4847
}
4948
else if (Path.IsPathRooted(traceEnvar) && // Trace to a file
50-
TryCreateTextWriter(context, traceEnvar, out var fileWriter))
49+
TryCreateTextWriter(_context, traceEnvar, out var fileWriter))
5150
{
52-
context.Trace.AddListener(fileWriter);
51+
_traceFileWriter = fileWriter;
52+
_context.Trace.AddListener(fileWriter);
5353
}
5454
else
5555
{
56-
context.StdError.WriteLine($"warning: cannot write trace output to {traceEnvar}");
56+
_context.StdError.WriteLine($"warning: cannot write trace output to {traceEnvar}");
5757
}
5858
}
5959

6060
// Enable sensitive tracing and show warning
61-
if (context.IsEnvironmentVariableTruthy(Constants.EnvironmentVariables.GcmTraceSecrets, false))
61+
if (_context.IsEnvironmentVariableTruthy(Constants.EnvironmentVariables.GcmTraceSecrets, false))
6262
{
63-
context.Trace.EnableSecretTracing = true;
64-
context.StdError.WriteLine(
65-
"warning: Secret tracing is enabled. Trace output may contain sensitive information.");
63+
_context.Trace.EnableSecretTracing = true;
64+
_context.StdError.WriteLine("Secret tracing is enabled. Trace output may contain sensitive information.");
6665
}
6766

6867
// Register all supported host providers
69-
HostProviderRegistry.Register(
70-
new AzureReposHostProvider(context),
71-
new GenericHostProvider(context)
68+
_providerRegistry.Register(
69+
new AzureReposHostProvider(_context),
70+
new GenericHostProvider(_context)
7271
);
7372

73+
// Construct all supported commands
74+
var commands = new CommandBase[]
75+
{
76+
new EraseCommand(_providerRegistry),
77+
new GetCommand(_providerRegistry),
78+
new StoreCommand(_providerRegistry),
79+
new VersionCommand(),
80+
new HelpCommand(),
81+
};
82+
7483
// Trace the current version and program arguments
75-
context.Trace.WriteLine($"{Constants.GetProgramHeader()} '{string.Join(" ", args)}'");
84+
_context.Trace.WriteLine($"{Constants.GetProgramHeader()} '{string.Join(" ", args)}'");
7685

7786
if (args.Length == 0)
7887
{
79-
context.StdError.WriteLine("Missing command.");
80-
HelpCommand.PrintUsage(context.StdError);
88+
_context.StdError.WriteLine("Missing command.");
89+
HelpCommand.PrintUsage(_context.StdError);
8190
return -1;
8291
}
8392

84-
foreach (var cmd in Commands)
93+
foreach (var cmd in commands)
8594
{
8695
if (cmd.CanExecute(args))
8796
{
8897
try
8998
{
90-
await cmd.ExecuteAsync(context, args);
99+
await cmd.ExecuteAsync(_context, args);
91100
return 0;
92101
}
93102
catch (Exception e)
94103
{
95104
if (e is AggregateException ae)
96105
{
97-
ae.Handle(x => WriteException(context, x));
106+
ae.Handle(x => WriteException(_context, x));
98107
}
99108
else
100109
{
101-
WriteException(context, e);
110+
WriteException(_context, e);
102111
}
103112

104113
return -1;
105114
}
106115
}
107116
}
108117

109-
context.StdError.WriteLine("Unrecognized command '{0}'.", args[0]);
110-
HelpCommand.PrintUsage(context.StdError);
118+
_context.StdError.WriteLine("Unrecognized command '{0}'.", args[0]);
119+
HelpCommand.PrintUsage(_context.StdError);
111120
return -1;
112121
}
113122

123+
#region Helpers
124+
114125
private static bool WriteException(ICommandContext context, Exception e)
115126
{
116127
context.StdError.WriteLine("fatal: {0}", e.Message);
@@ -140,5 +151,17 @@ private static bool TryCreateTextWriter(ICommandContext context, string path, ou
140151

141152
return writer != null;
142153
}
154+
155+
#endregion
156+
157+
#region IDisposable
158+
159+
public void Dispose()
160+
{
161+
_providerRegistry.Dispose();
162+
_traceFileWriter?.Dispose();
163+
}
164+
165+
#endregion
143166
}
144167
}

src/git-credential-manager/Program.cs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,11 @@ public static class Program
88
{
99
public static void Main(string[] args)
1010
{
11-
int exitCode = Application.RunAsync(args).ConfigureAwait(false).GetAwaiter().GetResult();
12-
Environment.Exit(exitCode);
11+
using (var app = new Application(new CommandContext()))
12+
{
13+
int exitCode = app.RunAsync(args).ConfigureAwait(false).GetAwaiter().GetResult();
14+
Environment.Exit(exitCode);
15+
}
1316
}
1417
}
1518
}

tests/Microsoft.Git.CredentialManager.Tests/Objects/TestHostProvider.cs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,5 +25,11 @@ public class TestHostProvider : IHostProvider
2525
Task<GitCredential> IHostProvider.CreateCredentialAsync(InputArguments input) => Task.FromResult(Credential);
2626

2727
#endregion
28+
29+
#region IDisposable
30+
31+
public void Dispose() { }
32+
33+
#endregion
2834
}
2935
}

tests/Microsoft.Git.CredentialManager.Tests/Objects/TestHostProviderRegistry.cs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,5 +18,10 @@ IHostProvider IHostProviderRegistry.GetProvider(InputArguments input)
1818
}
1919

2020
#endregion
21+
22+
public void Dispose()
23+
{
24+
Provider?.Dispose();
25+
}
2126
}
2227
}

0 commit comments

Comments
 (0)