Skip to content

Commit 9334312

Browse files
committed
Avoid event-based memory leak in case of exception in Authenticate.
Simplify Dispose(bool).
1 parent 1ce21e4 commit 9334312

File tree

5 files changed

+64
-101
lines changed

5 files changed

+64
-101
lines changed

src/Renci.SshNet/NoneAuthenticationMethod.cs

Lines changed: 17 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ namespace Renci.SshNet
1212
public class NoneAuthenticationMethod : AuthenticationMethod, IDisposable
1313
{
1414
private AuthenticationResult _authenticationResult = AuthenticationResult.Failure;
15-
1615
private EventWaitHandle _authenticationCompleted = new AutoResetEvent(false);
1716

1817
/// <summary>
@@ -31,7 +30,6 @@ public override string Name
3130
public NoneAuthenticationMethod(string username)
3231
: base(username)
3332
{
34-
3533
}
3634

3735
/// <summary>
@@ -50,12 +48,16 @@ public override AuthenticationResult Authenticate(Session session)
5048
session.UserAuthenticationSuccessReceived += Session_UserAuthenticationSuccessReceived;
5149
session.UserAuthenticationFailureReceived += Session_UserAuthenticationFailureReceived;
5250

53-
session.SendMessage(new RequestMessageNone(ServiceName.Connection, Username));
54-
55-
session.WaitOnHandle(_authenticationCompleted);
56-
57-
session.UserAuthenticationSuccessReceived -= Session_UserAuthenticationSuccessReceived;
58-
session.UserAuthenticationFailureReceived -= Session_UserAuthenticationFailureReceived;
51+
try
52+
{
53+
session.SendMessage(new RequestMessageNone(ServiceName.Connection, Username));
54+
session.WaitOnHandle(_authenticationCompleted);
55+
}
56+
finally
57+
{
58+
session.UserAuthenticationSuccessReceived -= Session_UserAuthenticationSuccessReceived;
59+
session.UserAuthenticationFailureReceived -= Session_UserAuthenticationFailureReceived;
60+
}
5961

6062
return _authenticationResult;
6163
}
@@ -90,7 +92,6 @@ private void Session_UserAuthenticationFailureReceived(object sender, MessageEve
9092
public void Dispose()
9193
{
9294
Dispose(true);
93-
9495
GC.SuppressFinalize(this);
9596
}
9697

@@ -100,22 +101,17 @@ public void Dispose()
100101
/// <param name="disposing"><c>true</c> to release both managed and unmanaged resources; <c>false</c> to release only unmanaged resources.</param>
101102
protected virtual void Dispose(bool disposing)
102103
{
103-
// Check to see if Dispose has already been called.
104-
if (!_isDisposed)
104+
if (_isDisposed)
105+
return;
106+
107+
if (disposing)
105108
{
106-
// If disposing equals true, dispose all managed
107-
// and unmanaged resources.
108-
if (disposing)
109+
if (_authenticationCompleted != null)
109110
{
110-
// Dispose managed resources.
111-
if (_authenticationCompleted != null)
112-
{
113-
_authenticationCompleted.Dispose();
114-
_authenticationCompleted = null;
115-
}
111+
_authenticationCompleted.Dispose();
112+
_authenticationCompleted = null;
116113
}
117114

118-
// Note disposing has been done.
119115
_isDisposed = true;
120116
}
121117
}

src/Renci.SshNet/PasswordAuthenticationMethod.cs

Lines changed: 20 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -12,18 +12,13 @@ namespace Renci.SshNet
1212
/// <summary>
1313
/// Provides functionality to perform password authentication.
1414
/// </summary>
15-
public partial class PasswordAuthenticationMethod : AuthenticationMethod, IDisposable
15+
public class PasswordAuthenticationMethod : AuthenticationMethod, IDisposable
1616
{
1717
private AuthenticationResult _authenticationResult = AuthenticationResult.Failure;
18-
1918
private Session _session;
20-
2119
private EventWaitHandle _authenticationCompleted = new AutoResetEvent(false);
22-
2320
private Exception _exception;
24-
2521
private readonly RequestMessage _requestMessage;
26-
2722
private readonly byte[] _password;
2823

2924
/// <summary>
@@ -65,7 +60,6 @@ public PasswordAuthenticationMethod(string username, byte[] password)
6560
throw new ArgumentNullException("password");
6661

6762
_password = password;
68-
6963
_requestMessage = new RequestMessagePassword(ServiceName.Connection, Username, _password);
7064
}
7165

@@ -88,21 +82,21 @@ public override AuthenticationResult Authenticate(Session session)
8882
session.UserAuthenticationFailureReceived += Session_UserAuthenticationFailureReceived;
8983
session.MessageReceived += Session_MessageReceived;
9084

91-
session.RegisterMessage("SSH_MSG_USERAUTH_PASSWD_CHANGEREQ");
92-
93-
session.SendMessage(_requestMessage);
94-
95-
session.WaitOnHandle(_authenticationCompleted);
96-
97-
session.UserAuthenticationSuccessReceived -= Session_UserAuthenticationSuccessReceived;
98-
session.UserAuthenticationFailureReceived -= Session_UserAuthenticationFailureReceived;
99-
session.MessageReceived -= Session_MessageReceived;
100-
85+
try
86+
{
87+
session.RegisterMessage("SSH_MSG_USERAUTH_PASSWD_CHANGEREQ");
88+
session.SendMessage(_requestMessage);
89+
session.WaitOnHandle(_authenticationCompleted);
90+
}
91+
finally
92+
{
93+
session.UserAuthenticationSuccessReceived -= Session_UserAuthenticationSuccessReceived;
94+
session.UserAuthenticationFailureReceived -= Session_UserAuthenticationFailureReceived;
95+
session.MessageReceived -= Session_MessageReceived;
96+
}
10197

10298
if (_exception != null)
103-
{
10499
throw _exception;
105-
}
106100

107101
return _authenticationResult;
108102
}
@@ -167,7 +161,6 @@ private void Session_MessageReceived(object sender, MessageEventArgs<Message> e)
167161
public void Dispose()
168162
{
169163
Dispose(true);
170-
171164
GC.SuppressFinalize(this);
172165
}
173166

@@ -177,22 +170,17 @@ public void Dispose()
177170
/// <param name="disposing"><c>true</c> to release both managed and unmanaged resources; <c>false</c> to release only unmanaged resources.</param>
178171
protected virtual void Dispose(bool disposing)
179172
{
180-
// Check to see if Dispose has already been called.
181-
if (!_isDisposed)
173+
if (_isDisposed)
174+
return;
175+
176+
if (disposing)
182177
{
183-
// If disposing equals true, dispose all managed
184-
// and unmanaged resources.
185-
if (disposing)
178+
if (_authenticationCompleted != null)
186179
{
187-
// Dispose managed resources.
188-
if (_authenticationCompleted != null)
189-
{
190-
_authenticationCompleted.Dispose();
191-
_authenticationCompleted = null;
192-
}
180+
_authenticationCompleted.Dispose();
181+
_authenticationCompleted = null;
193182
}
194183

195-
// Note disposing has been done.
196184
_isDisposed = true;
197185
}
198186
}

src/Renci.SshNet/PasswordConnectionInfo.cs

Lines changed: 7 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@ public class PasswordConnectionInfo : ConnectionInfo, IDisposable
3737
public PasswordConnectionInfo(string host, string username, string password)
3838
: this(host, DefaultPort, username, Encoding.UTF8.GetBytes(password))
3939
{
40-
4140
}
4241

4342
/// <summary>
@@ -140,7 +139,6 @@ public PasswordConnectionInfo(string host, string username, string password, Pro
140139
public PasswordConnectionInfo(string host, string username, byte[] password)
141140
: this(host, DefaultPort, username, password)
142141
{
143-
144142
}
145143

146144
/// <summary>
@@ -273,7 +271,6 @@ private void AuthenticationMethod_PasswordExpired(object sender, AuthenticationP
273271
public void Dispose()
274272
{
275273
Dispose(true);
276-
277274
GC.SuppressFinalize(this);
278275
}
279276

@@ -283,24 +280,19 @@ public void Dispose()
283280
/// <param name="disposing"><c>true</c> to release both managed and unmanaged resources; <c>false</c> to release only unmanaged resources.</param>
284281
protected virtual void Dispose(bool disposing)
285282
{
286-
// Check to see if Dispose has already been called.
287-
if (!_isDisposed)
283+
if (_isDisposed)
284+
return;
285+
286+
if (disposing)
288287
{
289-
// If disposing equals true, dispose all managed
290-
// and unmanaged resources.
291-
if (disposing)
288+
if (AuthenticationMethods != null)
292289
{
293-
// Dispose managed resources.
294-
if (AuthenticationMethods != null)
290+
foreach (var authenticationMethods in AuthenticationMethods.OfType<IDisposable>())
295291
{
296-
foreach (var authenticationMethods in AuthenticationMethods.OfType<IDisposable>())
297-
{
298-
authenticationMethods.Dispose();
299-
}
292+
authenticationMethods.Dispose();
300293
}
301294
}
302295

303-
// Note disposing has been done.
304296
_isDisposed = true;
305297
}
306298
}

src/Renci.SshNet/PrivateKeyAuthenticationMethod.cs

Lines changed: 7 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,7 @@ namespace Renci.SshNet
1515
public class PrivateKeyAuthenticationMethod : AuthenticationMethod, IDisposable
1616
{
1717
private AuthenticationResult _authenticationResult = AuthenticationResult.Failure;
18-
1918
private EventWaitHandle _authenticationCompleted = new ManualResetEvent(false);
20-
2119
private bool _isSignatureRequired;
2220

2321
/// <summary>
@@ -154,7 +152,6 @@ private void Session_MessageReceived(object sender, MessageEventArgs<Message> e)
154152
public void Dispose()
155153
{
156154
Dispose(true);
157-
158155
GC.SuppressFinalize(this);
159156
}
160157

@@ -164,22 +161,17 @@ public void Dispose()
164161
/// <param name="disposing"><c>true</c> to release both managed and unmanaged resources; <c>false</c> to release only unmanaged resources.</param>
165162
protected virtual void Dispose(bool disposing)
166163
{
167-
// Check to see if Dispose has already been called.
168-
if (!_isDisposed)
164+
if (_isDisposed)
165+
return;
166+
167+
if (disposing)
169168
{
170-
// If disposing equals true, dispose all managed
171-
// and unmanaged resources.
172-
if (disposing)
169+
if (_authenticationCompleted != null)
173170
{
174-
// Dispose managed resources.
175-
if (_authenticationCompleted != null)
176-
{
177-
_authenticationCompleted.Dispose();
178-
_authenticationCompleted = null;
179-
}
171+
_authenticationCompleted.Dispose();
172+
_authenticationCompleted = null;
180173
}
181174

182-
// Note disposing has been done.
183175
_isDisposed = true;
184176
}
185177
}

src/Renci.SshNet/PrivateKeyConnectionInfo.cs

Lines changed: 13 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ public class PrivateKeyConnectionInfo : ConnectionInfo, IDisposable
2929
/// <code source="..\..\Renci.SshNet.Tests\Classes\PrivateKeyConnectionInfoTest.cs" region="Example PrivateKeyConnectionInfo PrivateKeyFile Multiple" language="C#" title="Connect using multiple private key" />
3030
/// </example>
3131
public PrivateKeyConnectionInfo(string host, string username, params PrivateKeyFile[] keyFiles)
32-
: this(host, ConnectionInfo.DefaultPort, username, ProxyTypes.None, string.Empty, 0, string.Empty, string.Empty, keyFiles)
32+
: this(host, DefaultPort, username, ProxyTypes.None, string.Empty, 0, string.Empty, string.Empty, keyFiles)
3333
{
3434

3535
}
@@ -87,7 +87,7 @@ public PrivateKeyConnectionInfo(string host, int port, string username, ProxyTyp
8787
/// <param name="proxyPort">The proxy port.</param>
8888
/// <param name="keyFiles">The key files.</param>
8989
public PrivateKeyConnectionInfo(string host, string username, ProxyTypes proxyType, string proxyHost, int proxyPort, params PrivateKeyFile[] keyFiles)
90-
: this(host, ConnectionInfo.DefaultPort, username, proxyType, proxyHost, proxyPort, string.Empty, string.Empty, keyFiles)
90+
: this(host, DefaultPort, username, proxyType, proxyHost, proxyPort, string.Empty, string.Empty, keyFiles)
9191
{
9292
}
9393

@@ -102,7 +102,7 @@ public PrivateKeyConnectionInfo(string host, string username, ProxyTypes proxyTy
102102
/// <param name="proxyUsername">The proxy username.</param>
103103
/// <param name="keyFiles">The key files.</param>
104104
public PrivateKeyConnectionInfo(string host, string username, ProxyTypes proxyType, string proxyHost, int proxyPort, string proxyUsername, params PrivateKeyFile[] keyFiles)
105-
: this(host, ConnectionInfo.DefaultPort, username, proxyType, proxyHost, proxyPort, proxyUsername, string.Empty, keyFiles)
105+
: this(host, DefaultPort, username, proxyType, proxyHost, proxyPort, proxyUsername, string.Empty, keyFiles)
106106
{
107107
}
108108

@@ -118,7 +118,7 @@ public PrivateKeyConnectionInfo(string host, string username, ProxyTypes proxyTy
118118
/// <param name="proxyPassword">The proxy password.</param>
119119
/// <param name="keyFiles">The key files.</param>
120120
public PrivateKeyConnectionInfo(string host, string username, ProxyTypes proxyType, string proxyHost, int proxyPort, string proxyUsername, string proxyPassword, params PrivateKeyFile[] keyFiles)
121-
: this(host, ConnectionInfo.DefaultPort, username, proxyType, proxyHost, proxyPort, proxyUsername, proxyPassword, keyFiles)
121+
: this(host, DefaultPort, username, proxyType, proxyHost, proxyPort, proxyUsername, proxyPassword, keyFiles)
122122
{
123123
}
124124

@@ -137,7 +137,7 @@ public PrivateKeyConnectionInfo(string host, string username, ProxyTypes proxyTy
137137
public PrivateKeyConnectionInfo(string host, int port, string username, ProxyTypes proxyType, string proxyHost, int proxyPort, string proxyUsername, string proxyPassword, params PrivateKeyFile[] keyFiles)
138138
: base(host, port, username, proxyType, proxyHost, proxyPort, proxyUsername, proxyPassword, new PrivateKeyAuthenticationMethod(username, keyFiles))
139139
{
140-
this.KeyFiles = new Collection<PrivateKeyFile>(keyFiles);
140+
KeyFiles = new Collection<PrivateKeyFile>(keyFiles);
141141
}
142142

143143
#region IDisposable Members
@@ -150,7 +150,6 @@ public PrivateKeyConnectionInfo(string host, int port, string username, ProxyTyp
150150
public void Dispose()
151151
{
152152
Dispose(true);
153-
154153
GC.SuppressFinalize(this);
155154
}
156155

@@ -160,24 +159,20 @@ public void Dispose()
160159
/// <param name="disposing"><c>true</c> to release both managed and unmanaged resources; <c>false</c> to release only unmanaged resources.</param>
161160
protected virtual void Dispose(bool disposing)
162161
{
163-
// Check to see if Dispose has already been called.
164-
if (!this._isDisposed)
162+
if (_isDisposed)
163+
return;
164+
165+
if (disposing)
165166
{
166-
// If disposing equals true, dispose all managed
167-
// and unmanaged resources.
168-
if (disposing)
167+
// Dispose managed resources.
168+
if (AuthenticationMethods != null)
169169
{
170-
// Dispose managed resources.
171-
if (this.AuthenticationMethods != null)
170+
foreach (var authenticationMethods in AuthenticationMethods.OfType<IDisposable>())
172171
{
173-
foreach (var authenticationMethods in this.AuthenticationMethods.OfType<IDisposable>())
174-
{
175-
authenticationMethods.Dispose();
176-
}
172+
authenticationMethods.Dispose();
177173
}
178174
}
179175

180-
// Note disposing has been done.
181176
_isDisposed = true;
182177
}
183178
}

0 commit comments

Comments
 (0)