patterncsharpMinor
Async TcpListener
Viewed 0 times
asynctcplistenerstackoverflow
Problem
I'm trying to implement a TcpListener using the TPL. I'm not sure If I'm doing everything correctly though. I'm trying to keep the code as small as possible as well. It all works fine in a console application.
Here is the
And the
```
private async Task ReadStringAsync(ConnectionState connection, Encoding encoding, IProgress progress)
{
var stream = connection.TcpClient.GetStream();
if (connection.Read > 0)
{
var encoded = encoding.GetString(connection.Buffer, 0, connection.Read);
connection.StringBuilder.Append(encoded);
}
var decoded = connection.StringBuilder.ToString();
if (decoded.Contains(""))
{
progress.Report(decoded.Replace("", string.Empty));
}
connection.Read = await stream.ReadAsync(connection.Buffer, 0, connecti
public async Task RunAsync(CancellationToken cancellationToken)
{
var listener = new TcpListener(ip, port);
listener.Start();
while (!cancellationToken.IsCancellationRequested)
{
await AcceptClientAsync(listener, encoding, progress, cancellationToken);
}
listener.Stop();
}Here is the
AcceptClientAsync method: (Sorry for the weird formatting. StyleCop likes to do that.)private async Task AcceptClientAsync(TcpListener tcpListener, Encoding encoding, IProgress progress, CancellationToken cancellationToken)
{
var client = await tcpListener.AcceptTcpClientAsync();
this.Connection = new ConnectionState { TcpClient = client };
while (client.Connected && !cancellationToken.IsCancellationRequested)
{
await
this.ReadStringAsync(this.Connection, encoding, progress)
.ContinueWith(
task =>
progress.Report(
string.Format("Client {0} disconnected.", Connection.TcpClient.Client.RemoteEndPoint)),
TaskContinuationOptions.OnlyOnFaulted);
}
}And the
ReadStringAsync method:```
private async Task ReadStringAsync(ConnectionState connection, Encoding encoding, IProgress progress)
{
var stream = connection.TcpClient.GetStream();
if (connection.Read > 0)
{
var encoded = encoding.GetString(connection.Buffer, 0, connection.Read);
connection.StringBuilder.Append(encoded);
}
var decoded = connection.StringBuilder.ToString();
if (decoded.Contains(""))
{
progress.Report(decoded.Replace("", string.Empty));
}
connection.Read = await stream.ReadAsync(connection.Buffer, 0, connecti
Solution
-
If you immediately
-
Because of the way TCP works, the
-
-
I think the way you're dealing with `
If you immediately
await everything, you're not going to get any concurrency. This means that at any moment, there can be only one connection to this listener. Is this intentional?-
Because of the way TCP works, the
Connected property may return true even if the other side already disconnected. This means that you should send keep-alive packets, even if all you logically want to do is reading.-
ContinueWith() is useful only rarely when you can use async. OnlyOnFaulted can be easily rewritten using try-catch. Though you should be catching only the specific exception that you need, not all of them. So your code could look something like:try
{
await this.ReadStringAsync(this.Connection, encoding, progress);
}
catch (NotSureWhichException ex)
{
progress.Report(
string.Format("Client {0} disconnected.", Connection.TcpClient.Client.RemoteEndPoint));
}-
I think the way you're dealing with `
is wrong. If there is always going to be only one message per connection, then you should somehow indicate that after reading . If there can be multiple messages, then an end of one message and start of the following one could be read in the same ReadAsync()`, which means you're not going to report partial message.Code Snippets
try
{
await this.ReadStringAsync(this.Connection, encoding, progress);
}
catch (NotSureWhichException ex)
{
progress.Report(
string.Format("Client {0} disconnected.", Connection.TcpClient.Client.RemoteEndPoint));
}Context
StackExchange Code Review Q#42480, answer score: 4
Revisions (0)
No revisions yet.