Handle case where scanning happens after dispose. (#475)

- ConnectionManager.Scan would null ref because the timer.Change was being called
after closing all connections.
This commit is contained in:
David Fowler 2017-05-21 23:19:17 -07:00 committed by GitHub
parent 87c4da41e8
commit 323dae4ce9
2 changed files with 17 additions and 7 deletions

View File

@ -66,8 +66,7 @@ namespace Microsoft.AspNetCore.Sockets
public void RemoveConnection(string id) public void RemoveConnection(string id)
{ {
ConnectionState state; if (_connections.TryRemove(id, out _))
if (_connections.TryRemove(id, out state))
{ {
// Remove the connection completely // Remove the connection completely
_logger.LogDebug("Removing {connectionId} from the list of connections", id); _logger.LogDebug("Removing {connectionId} from the list of connections", id);
@ -85,9 +84,9 @@ namespace Microsoft.AspNetCore.Sockets
((ConnectionManager)state).Scan(); ((ConnectionManager)state).Scan();
} }
private void Scan() public void Scan()
{ {
// If we couldn't get the lock it means one of 2 things is true: // If we couldn't get the lock it means one of 2 things are true:
// - We're about to dispose so we don't care to run the scan callback anyways. // - We're about to dispose so we don't care to run the scan callback anyways.
// - The previous Scan took long enough that the next scan tried to run in parallel // - The previous Scan took long enough that the next scan tried to run in parallel
// In either case just do nothing and end the timer callback as soon as possible // In either case just do nothing and end the timer callback as soon as possible
@ -132,12 +131,12 @@ namespace Microsoft.AspNetCore.Sockets
var ignore = DisposeAndRemoveAsync(c.Value); var ignore = DisposeAndRemoveAsync(c.Value);
} }
} }
// Resume once we finished processing all connections
_timer.Change(TimeSpan.FromSeconds(1), TimeSpan.FromSeconds(1));
} }
finally finally
{ {
// Resume once we finished processing all connections
_timer.Change(TimeSpan.FromSeconds(1), TimeSpan.FromSeconds(1));
// Exit the lock now // Exit the lock now
Monitor.Exit(_executionLock); Monitor.Exit(_executionLock);
} }

View File

@ -181,6 +181,17 @@ namespace Microsoft.AspNetCore.Sockets.Tests
Assert.Equal(ConnectionState.ConnectionStatus.Disposed, state.Status); Assert.Equal(ConnectionState.ConnectionStatus.Disposed, state.Status);
} }
[Fact]
public void ScanAfterDisposeNoops()
{
var connectionManager = CreateConnectionManager();
var state = connectionManager.CreateConnection();
connectionManager.CloseConnections();
connectionManager.Scan();
}
private static ConnectionManager CreateConnectionManager() private static ConnectionManager CreateConnectionManager()
{ {
return new ConnectionManager(new Logger<ConnectionManager>(new LoggerFactory())); return new ConnectionManager(new Logger<ConnectionManager>(new LoggerFactory()));