提交 58ed8f6c 编写于 作者: J Jared Parsons

Merge pull request #3022 from jaredpar/fix-2866

Fix connection issues in server
......@@ -14,6 +14,18 @@ namespace Microsoft.CodeAnalysis.CompilerServer
{
internal partial class ServerDispatcher
{
internal struct ConnectionData
{
public readonly CompletionReason CompletionReason;
public readonly TimeSpan? KeepAlive;
public ConnectionData(CompletionReason completionReason, TimeSpan? keepAlive = null)
{
CompletionReason = completionReason;
KeepAlive = keepAlive;
}
}
internal enum CompletionReason
{
/// <summary>
......@@ -52,9 +64,8 @@ public Connection(IClientConnection clientConnection, IRequestHandler handler)
_handler = handler;
}
public async Task<CompletionReason> ServeConnection(TaskCompletionSource<TimeSpan?> timeoutCompletionSource = null, CancellationToken cancellationToken = default(CancellationToken))
public async Task<ConnectionData> ServeConnection(CancellationToken cancellationToken = default(CancellationToken))
{
timeoutCompletionSource = timeoutCompletionSource ?? new TaskCompletionSource<TimeSpan?>();
try
{
BuildRequest request;
......@@ -67,10 +78,10 @@ public async Task<CompletionReason> ServeConnection(TaskCompletionSource<TimeSpa
catch (Exception e)
{
LogException(e, "Error reading build request.");
return CompletionReason.CompilationNotStarted;
return new ConnectionData(CompletionReason.CompilationNotStarted);
}
CheckForNewKeepAlive(request, timeoutCompletionSource);
var keepAlive = CheckForNewKeepAlive(request);
// Kick off both the compilation and a task to monitor the pipe for closing.
var buildCts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken);
......@@ -105,15 +116,11 @@ public async Task<CompletionReason> ServeConnection(TaskCompletionSource<TimeSpa
// Begin the tear down of the Task which didn't complete.
buildCts.Cancel();
return reason;
return new ConnectionData(reason, keepAlive);
}
finally
{
_clientConnection.Close();
// Ensure that the task is completed. If the code previously added a real result this will
// simply be a nop.
timeoutCompletionSource.TrySetResult(null);
}
}
......@@ -121,7 +128,7 @@ public async Task<CompletionReason> ServeConnection(TaskCompletionSource<TimeSpa
/// Check the request arguments for a new keep alive time. If one is present,
/// set the server timer to the new time.
/// </summary>
private void CheckForNewKeepAlive(BuildRequest request, TaskCompletionSource<TimeSpan?> timeoutCompletionSource)
private TimeSpan? CheckForNewKeepAlive(BuildRequest request)
{
TimeSpan? timeout = null;
foreach (var arg in request.Arguments)
......@@ -140,7 +147,7 @@ private void CheckForNewKeepAlive(BuildRequest request, TaskCompletionSource<Tim
}
}
timeoutCompletionSource.SetResult(timeout);
return timeout;
}
private Task<BuildResponse> ServeBuildRequest(BuildRequest request, CancellationToken cancellationToken)
......
......@@ -34,18 +34,6 @@ internal interface IRequestHandler
/// </remarks>
internal partial class ServerDispatcher
{
private class ConnectionData
{
public readonly Task<CompletionReason> ConnectionTask;
public Task<TimeSpan?> ChangeKeepAliveTask;
internal ConnectionData(Task<CompletionReason> connectionTask, Task<TimeSpan?> changeKeepAliveTask)
{
ConnectionTask = connectionTask;
ChangeKeepAliveTask = changeKeepAliveTask;
}
}
/// <summary>
/// Default time the server will stay alive after the last request disconnects.
/// </summary>
......@@ -154,15 +142,15 @@ public ServerDispatcher(IRequestHandler handler, IDiagnosticListener diagnosticL
/// test framework. The code hooks <see cref="AppDomain.AssemblyResolve"/> in a way
/// that prevents xUnit from running correctly and hence must be disabled.
/// </remarks>
[System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Reliability", "CA2001:AvoidCallingProblematicMethods",
MessageId = "System.GC.Collect",
Justification ="We intentionally call GC.Collect when anticipate long period on inactivity.")]
[System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Reliability", "CA2001:AvoidCallingProblematicMethods",
MessageId = "System.GC.Collect",
Justification = "We intentionally call GC.Collect when anticipate long period on inactivity.")]
public void ListenAndDispatchConnections(string pipeName, TimeSpan? keepAlive, CancellationToken cancellationToken = default(CancellationToken))
{
Debug.Assert(SynchronizationContext.Current == null);
var isKeepAliveDefault = true;
var connectionList = new List<ConnectionData>();
var connectionList = new List<Task<ConnectionData>>();
Task gcTask = null;
Task timeoutTask = null;
Task<NamedPipeServerStream> listenTask = null;
......@@ -192,9 +180,8 @@ public void ListenAndDispatchConnections(string pipeName, TimeSpan? keepAlive, C
// If there is a connection event that has highest priority.
if (listenTask.IsCompleted && !cancellationToken.IsCancellationRequested)
{
var changeKeepAliveSource = new TaskCompletionSource<TimeSpan?>();
var connectionTask = CreateHandleConnectionTask(listenTask, changeKeepAliveSource, cancellationToken);
connectionList.Add(new ConnectionData(connectionTask, changeKeepAliveSource.Task));
var connectionTask = CreateHandleConnectionTask(listenTask, _handler, cancellationToken);
connectionList.Add(connectionTask);
listenTask = null;
listenCancellationTokenSource = null;
timeoutTask = null;
......@@ -234,7 +221,7 @@ public void ListenAndDispatchConnections(string pipeName, TimeSpan? keepAlive, C
try
{
Task.WaitAll(connectionList.Select(x => x.ConnectionTask).ToArray());
Task.WaitAll(connectionList.ToArray());
}
catch
{
......@@ -248,11 +235,10 @@ public void ListenAndDispatchConnections(string pipeName, TimeSpan? keepAlive, C
/// The server farms out work to Task values and this method needs to wait until at least one of them
/// has completed.
/// </summary>
private void WaitForAnyCompletion(IEnumerable<ConnectionData> e, Task[] other, CancellationToken cancellationToken)
private void WaitForAnyCompletion(IEnumerable<Task<ConnectionData>> e, Task[] other, CancellationToken cancellationToken)
{
var all = new List<Task>();
all.AddRange(e.Select(x => x.ConnectionTask));
all.AddRange(e.Select(x => x.ChangeKeepAliveTask).Where(x => x != null));
all.AddRange(e);
all.AddRange(other.Where(x => x != null));
try
......@@ -270,32 +256,31 @@ private void WaitForAnyCompletion(IEnumerable<ConnectionData> e, Task[] other, C
/// Checks the completed connection objects.
/// </summary>
/// <returns>True if everything completed normally and false if there were any client disconnections.</returns>
private bool CheckConnectionTask(List<ConnectionData> connectionList, ref TimeSpan? keepAlive, ref bool isKeepAliveDefault)
private bool CheckConnectionTask(List<Task<ConnectionData>> connectionList, ref TimeSpan? keepAlive, ref bool isKeepAliveDefault)
{
var allFine = true;
foreach (var current in connectionList)
var processedCount = 0;
var i = 0;
while (i < connectionList.Count)
{
if (current.ChangeKeepAliveTask != null && current.ChangeKeepAliveTask.IsCompleted)
var current = connectionList[i];
if (!current.IsCompleted)
{
ChangeKeepAlive(current.ChangeKeepAliveTask, ref keepAlive, ref isKeepAliveDefault);
current.ChangeKeepAliveTask = null;
i++;
continue;
}
if (current.ConnectionTask.IsCompleted)
{
// https://github.com/dotnet/roslyn/issues/2866
// Debug.Assert(current.ChangeKeepAliveTask == null);
connectionList.RemoveAt(i);
processedCount++;
if (current.ConnectionTask.Result == CompletionReason.ClientDisconnect)
{
allFine = false;
}
var connectionData = current.Result;
ChangeKeepAlive(connectionData.KeepAlive, ref keepAlive, ref isKeepAliveDefault);
if (connectionData.CompletionReason == CompletionReason.ClientDisconnect)
{
allFine = false;
}
}
// Finally remove any ConnectionData for connections which are no longer active.
int processedCount = connectionList.RemoveAll(x => x.ConnectionTask.IsCompleted);
if (processedCount > 0)
{
_diagnosticListener.ConnectionProcessed(processedCount);
......@@ -304,15 +289,8 @@ private bool CheckConnectionTask(List<ConnectionData> connectionList, ref TimeSp
return allFine;
}
private void ChangeKeepAlive(Task<TimeSpan?> task, ref TimeSpan? keepAlive, ref bool isKeepAliveDefault)
private void ChangeKeepAlive(TimeSpan? value, ref TimeSpan? keepAlive, ref bool isKeepAliveDefault)
{
Debug.Assert(task.IsCompleted);
if (task.Status != TaskStatus.RanToCompletion)
{
return;
}
var value = task.Result;
if (value.HasValue)
{
if (isKeepAliveDefault || !keepAlive.HasValue || value.Value > keepAlive.Value)
......@@ -407,15 +385,28 @@ private async Task<NamedPipeServerStream> CreateListenTask(string pipeName, Canc
}
/// <summary>
/// Creates a Task representing the processing of the new connection. Returns null
/// if the server is unable to create a new Task object for the connection.
/// Creates a Task representing the processing of the new connection. This will return a task that
/// will never fail. It will always produce a <see cref="ConnectionData"/> value. Connection errors
/// will end up being represented as <see cref="CompletionReason.ClientDisconnect"/>
/// </summary>
private async Task<CompletionReason> CreateHandleConnectionTask(Task<NamedPipeServerStream> pipeStreamTask, TaskCompletionSource<TimeSpan?> changeKeepAliveSource, CancellationToken cancellationToken)
internal static async Task<ConnectionData> CreateHandleConnectionTask(Task<NamedPipeServerStream> pipeStreamTask, IRequestHandler handler, CancellationToken cancellationToken)
{
var pipeStream = await pipeStreamTask.ConfigureAwait(false);
var clientConnection = new NamedPipeClientConnection(pipeStream);
var connection = new Connection(clientConnection, _handler);
return await connection.ServeConnection(changeKeepAliveSource, cancellationToken).ConfigureAwait(false);
Connection connection;
try
{
var pipeStream = await pipeStreamTask.ConfigureAwait(false);
var clientConnection = new NamedPipeClientConnection(pipeStream);
connection = new Connection(clientConnection, handler);
}
catch (Exception ex)
{
// Unable to establish a connection with the client. The client is responsible for
// handling this case. Nothing else for us to do here.
CompilerServerLogger.LogException(ex, "Error creating client named pipe");
return new ConnectionData(CompletionReason.ClientDisconnect);
}
return await connection.ServeConnection(cancellationToken).ConfigureAwait(false);
}
/// <summary>
......
......@@ -194,7 +194,7 @@ public void ClientDisconnectCancelBuildAndReturnsFailure()
.Returns(s_emptyBuildResponse);
var client = new ServerDispatcher.Connection(clientConnection, handler.Object);
var serveTask = client.ServeConnection(new TaskCompletionSource<TimeSpan?>());
var serveTask = client.ServeConnection();
// Once this returns we know the Connection object has kicked off a compilation and
// started monitoring the disconnect task. Can now initiate a disconnect in a known
......@@ -202,7 +202,7 @@ public void ClientDisconnectCancelBuildAndReturnsFailure()
var cancellationToken = handlerTaskSource.Task.Result;
monitorTaskSource.SetResult(true);
Assert.Equal(ServerDispatcher.CompletionReason.ClientDisconnect, serveTask.Result);
Assert.Equal(ServerDispatcher.CompletionReason.ClientDisconnect, serveTask.Result.CompletionReason);
Assert.True(cancellationToken.IsCancellationRequested);
// Now that the asserts are done unblock the "build" long running task. Have to do this
......@@ -221,7 +221,7 @@ public void ReadError()
clientConnection.CloseAction = delegate { calledClose = true; };
var client = new ServerDispatcher.Connection(clientConnection, handler.Object);
Assert.Equal(ServerDispatcher.CompletionReason.CompilationNotStarted, client.ServeConnection().Result);
Assert.Equal(ServerDispatcher.CompletionReason.CompilationNotStarted, client.ServeConnection().Result.CompletionReason);
Assert.True(calledClose);
}
......@@ -242,7 +242,7 @@ public void WriteError()
.Returns(s_emptyBuildResponse);
var client = new ServerDispatcher.Connection(clientConnection, handler.Object);
Assert.Equal(ServerDispatcher.CompletionReason.ClientDisconnect, client.ServeConnection().Result);
Assert.Equal(ServerDispatcher.CompletionReason.ClientDisconnect, client.ServeConnection().Result.CompletionReason);
}
[Fact]
......@@ -258,6 +258,19 @@ public void KeepAliveNoConnections()
Assert.True((DateTime.Now - startTime) > keepAlive);
}
[Fact]
public async Task FailedConnectionShoudlCreateFailedConnectionData()
{
var tcs = new TaskCompletionSource<NamedPipeServerStream>();
var handler = new Mock<IRequestHandler>(MockBehavior.Strict);
var connectionDataTask = ServerDispatcher.CreateHandleConnectionTask(tcs.Task, handler.Object, CancellationToken.None);
tcs.SetException(new Exception());
var connectionData = await connectionDataTask.ConfigureAwait(false);
Assert.Equal(ServerDispatcher.CompletionReason.ClientDisconnect, connectionData.CompletionReason);
Assert.Null(connectionData.KeepAlive);
}
/// <summary>
/// Ensure server respects keep alive and shuts down after processing a single connection.
/// </summary>
......
Markdown is supported
0% .
You are about to add 0 people to the discussion. Proceed with caution.
先完成此消息的编辑!
想要评论请 注册