-
Notifications
You must be signed in to change notification settings - Fork 323
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Event based communication over sockets. #1294
Changes from 14 commits
a6105f1
14a3693
8e985ce
7a39bb5
48c04da
ebf7546
8d60783
d19f759
06a759d
c75ea37
6189bc0
3d5db37
070ffa8
5ed4c8e
38a5e20
2fe9b5f
6341cf6
e4886e0
d066305
1dbd500
00f71c8
a464726
5c41c0d
1fe7cec
0db8785
79bd89c
fbb4922
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,32 @@ | ||
// Copyright (c) Microsoft Corporation. All rights reserved. | ||
// Licensed under the MIT license. See LICENSE file in the project root for full license information. | ||
|
||
namespace Microsoft.VisualStudio.TestPlatform.CommunicationUtilities.Interfaces | ||
{ | ||
using System; | ||
|
||
public interface ICommunicationEndPoint | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Doc? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sent it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can be put it in vstest-docs, once refined. |
||
{ | ||
/// <summary> | ||
/// Event raised when an endpoint is connected. | ||
/// </summary> | ||
event EventHandler<ConnectedEventArgs> Connected; | ||
|
||
/// <summary> | ||
/// Event raised when an endpoint is disconnected. | ||
/// </summary> | ||
event EventHandler<DisconnectedEventArgs> Disconnected; | ||
|
||
/// <summary> | ||
/// Starts the endpoint and channel. | ||
/// </summary> | ||
/// <param name="endpoint">Address to connect</param> | ||
/// <returns>Address of the connected endpoint</returns> | ||
string Start(string endpoint); | ||
|
||
/// <summary> | ||
/// Stops the endpoint and closes the underlying communication channel. | ||
/// </summary> | ||
void Stop(); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -57,10 +57,7 @@ public Task NotifyDataAvailable() | |
// connection is closed. | ||
var data = this.reader.ReadString(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's caught in MessageLoopAsync method. |
||
|
||
if (this.MessageReceived != null) | ||
{ | ||
this.MessageReceived.SafeInvoke(this, new MessageReceivedEventArgs { Data = data }, "LengthPrefixCommunicationChannel: MessageReceived"); | ||
} | ||
this.MessageReceived?.SafeInvoke(this, new MessageReceivedEventArgs { Data = data }, "LengthPrefixCommunicationChannel: MessageReceived"); | ||
|
||
return Task.FromResult(0); | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,7 +17,7 @@ namespace Microsoft.VisualStudio.TestPlatform.CommunicationUtilities | |
/// <summary> | ||
/// Communication client implementation over sockets. | ||
/// </summary> | ||
public class SocketClient : ICommunicationClient | ||
public class SocketClient : ICommunicationEndPoint | ||
{ | ||
private readonly CancellationTokenSource cancellation; | ||
private readonly TcpClient tcpClient; | ||
|
@@ -41,16 +41,19 @@ protected SocketClient(Func<Stream, ICommunicationChannel> channelFactory) | |
} | ||
|
||
/// <inheritdoc /> | ||
public event EventHandler<ConnectedEventArgs> ServerConnected; | ||
public event EventHandler<ConnectedEventArgs> Connected; | ||
|
||
/// <inheritdoc /> | ||
public event EventHandler<DisconnectedEventArgs> ServerDisconnected; | ||
public event EventHandler<DisconnectedEventArgs> Disconnected; | ||
|
||
/// <inheritdoc /> | ||
public void Start(string connectionInfo) | ||
public string Start(string endpoint) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
{ | ||
this.tcpClient.ConnectAsync(IPAddress.Loopback, int.Parse(connectionInfo)) | ||
.ContinueWith(this.OnServerConnected); | ||
var ipEndPoint = endpoint.GetIPEndPoint(); | ||
|
||
// Don't start if the endpoint port is zero | ||
this.tcpClient.ConnectAsync(ipEndPoint.Address, ipEndPoint.Port).ContinueWith(this.OnServerConnected); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add EqtTrace here saying that "Waiting for connecting to server". |
||
return endpoint; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should return There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
} | ||
|
||
/// <inheritdoc /> | ||
|
@@ -65,22 +68,24 @@ public void Stop() | |
|
||
private void OnServerConnected(Task connectAsyncTask) | ||
{ | ||
if (connectAsyncTask.IsFaulted) | ||
{ | ||
throw connectAsyncTask.Exception; | ||
} | ||
|
||
this.channel = this.channelFactory(this.tcpClient.GetStream()); | ||
if (this.ServerConnected != null) | ||
if (this.Connected != null) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can Connected ever be null in this class? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's an event so it can be null. But in our case it will not be null. |
||
{ | ||
this.ServerConnected.SafeInvoke(this, new ConnectedEventArgs(this.channel), "SocketClient: ServerConnected"); | ||
if (connectAsyncTask.IsFaulted) | ||
{ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add EqtTrace on failure and success scenarios. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
this.Connected.SafeInvoke(this, new ConnectedEventArgs(connectAsyncTask.Exception), "SocketClient: ServerConnected"); | ||
} | ||
else | ||
{ | ||
this.channel = this.channelFactory(this.tcpClient.GetStream()); | ||
this.Connected.SafeInvoke(this, new ConnectedEventArgs(this.channel), "SocketClient: ServerConnected"); | ||
|
||
// Start the message loop | ||
Task.Run(() => this.tcpClient.MessageLoopAsync( | ||
this.channel, | ||
this.Stop, | ||
this.cancellation.Token)) | ||
.ConfigureAwait(false); | ||
// Start the message loop | ||
Task.Run(() => this.tcpClient.MessageLoopAsync( | ||
this.channel, | ||
this.Stop, | ||
this.cancellation.Token)) | ||
.ConfigureAwait(false); | ||
} | ||
} | ||
} | ||
|
||
|
@@ -102,7 +107,7 @@ private void Stop(Exception error) | |
this.channel.Dispose(); | ||
this.cancellation.Dispose(); | ||
|
||
this.ServerDisconnected?.SafeInvoke(this, new DisconnectedEventArgs(), "SocketClient: ServerDisconnected"); | ||
this.Disconnected?.SafeInvoke(this, new DisconnectedEventArgs(), "SocketClient: ServerDisconnected"); | ||
} | ||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,7 +17,7 @@ namespace Microsoft.VisualStudio.TestPlatform.CommunicationUtilities | |
/// <summary> | ||
/// Communication server implementation over sockets. | ||
/// </summary> | ||
public class SocketServer : ICommunicationServer | ||
public class SocketServer : ICommunicationEndPoint | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Delete There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
{ | ||
private readonly CancellationTokenSource cancellation; | ||
|
||
|
@@ -35,7 +35,7 @@ public class SocketServer : ICommunicationServer | |
/// Initializes a new instance of the <see cref="SocketServer"/> class. | ||
/// </summary> | ||
public SocketServer() | ||
: this((stream) => new LengthPrefixCommunicationChannel(stream)) | ||
: this(stream => new LengthPrefixCommunicationChannel(stream)) | ||
{ | ||
} | ||
|
||
|
@@ -53,21 +53,19 @@ protected SocketServer(Func<Stream, ICommunicationChannel> channelFactory) | |
} | ||
|
||
/// <inheritdoc /> | ||
public event EventHandler<ConnectedEventArgs> ClientConnected; | ||
public event EventHandler<ConnectedEventArgs> Connected; | ||
|
||
/// <inheritdoc /> | ||
public event EventHandler<DisconnectedEventArgs> ClientDisconnected; | ||
public event EventHandler<DisconnectedEventArgs> Disconnected; | ||
|
||
/// <inheritdoc /> | ||
public string Start() | ||
public string Start(string endpoint) | ||
{ | ||
var endpoint = new IPEndPoint(IPAddress.Loopback, 0); | ||
this.tcpListener = new TcpListener(endpoint); | ||
this.tcpListener = new TcpListener(endpoint.GetIPEndPoint()); | ||
|
||
this.tcpListener.Start(); | ||
|
||
var connectionInfo = ((IPEndPoint)this.tcpListener.LocalEndpoint).Port.ToString(); | ||
EqtTrace.Info("SocketServer: Listening on port : {0}", connectionInfo); | ||
var connectionInfo = ((IPEndPoint)this.tcpListener.LocalEndpoint).ToString(); | ||
EqtTrace.Info("SocketServer: Listening on end point : {0}", connectionInfo); | ||
|
||
// Serves a single client at the moment. An error in connection, or message loop just | ||
// terminates the entire server. | ||
|
@@ -90,10 +88,10 @@ private void OnClientConnected(TcpClient client) | |
this.tcpClient = client; | ||
this.tcpClient.Client.NoDelay = true; | ||
|
||
if (this.ClientConnected != null) | ||
if (this.Connected != null) | ||
{ | ||
this.channel = this.channelFactory(this.tcpClient.GetStream()); | ||
this.ClientConnected.SafeInvoke(this, new ConnectedEventArgs(this.channel), "SocketServer: ClientConnected"); | ||
this.Connected.SafeInvoke(this, new ConnectedEventArgs(this.channel), "SocketServer: ClientConnected"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add EqtTrace on failure and success scenarios. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
|
||
// Start the message loop | ||
Task.Run(() => this.tcpClient.MessageLoopAsync(this.channel, error => this.Stop(error), this.cancellation.Token)).ConfigureAwait(false); | ||
|
@@ -121,7 +119,7 @@ private void Stop(Exception error) | |
this.channel.Dispose(); | ||
this.cancellation.Dispose(); | ||
|
||
this.ClientDisconnected?.SafeInvoke(this, new DisconnectedEventArgs { Error = error }, "SocketServer: ClientDisconnected"); | ||
this.Disconnected?.SafeInvoke(this, new DisconnectedEventArgs { Error = error }, "SocketServer: ClientDisconnected"); | ||
} | ||
} | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is communicationutilities namespace in common assembly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved the files.