3

I want to send data over tcp to a specific ip\port I've written a sample which should send there some string:

internal class TcpSender : BaseDataSender
{
    public TcpSender(Settings settings) : base(settings)
    {
    }

    public async override Task SendDataAsync(string data)
    {
        Guard.ArgumentNotNullOrEmptyString(data, nameof(data));

        byte[] sendData = Encoding.UTF8.GetBytes(data);
        using (var client = new TcpClient(Settings.IpAddress, Settings.Port))
        using (var stream = client.GetStream())
        {
            await stream.WriteAsync(sendData, 0, sendData.Length);
        }
    }
}

The issue here that my stream is disposed before tcp client have sent all the data. How should I rewrite my code to wait all data to be written and only after that dispose all resources? Thanks

UPD: called from console util:

static void Main(string[] args)
{
    // here settings and date are gotten from args
    try
    {
        GenerateAndSendData(settings, date)
                .GetAwaiter()
                .GetResult();
    }
    catch (Exception e)
    {
        Console.ForegroundColor = ConsoleColor.Red;
        Console.WriteLine(e);
    }
}

public static async Task GenerateAndSendData(Settings settings, DateTime date)
{
    var sender = new TcpSender(settings);
    await sender.SendDataAsync("Lorem Ipsum is simply dummy text of the printing and typesetting industry. Lorem Ipsum has been the industry's standard dummy text ever since the 1500s, when an unknown printer took a galley of type and scrambled it to make a type specimen book. It has survived not only five centuries, but also the leap into electronic typesetting, remaining essentially unchanged. It was popularised in the 1960s with the release of Letraset sheets containing Lorem Ipsum passages, and more recently with desktop publishing software like Aldus PageMaker including versions of Lorem Ipsum.");
}

Upd2: Echo server code (was stolen from some stackoverflow question):

class TcpEchoServer
{
    static TcpListener listen;
    static Thread serverthread;

    public static void Start()
    {
        listen = new TcpListener(System.Net.IPAddress.Parse("127.0.0.1"), 514);
        serverthread = new Thread(new ThreadStart(DoListen));
        serverthread.Start();
    }

    private static void DoListen()
    {
        // Listen
        listen.Start();
        Console.WriteLine("Server: Started server");

        while (true)
        {
            Console.WriteLine("Server: Waiting...");
            TcpClient client = listen.AcceptTcpClient();
            Console.WriteLine("Server: Waited");

            // New thread with client
            Thread clientThread = new Thread(new ParameterizedThreadStart(DoClient));
            clientThread.Start(client);
        }
    }

    private static void DoClient(object client)
    {
        // Read data
        TcpClient tClient = (TcpClient)client;

        Console.WriteLine("Client (Thread: {0}): Connected!", Thread.CurrentThread.ManagedThreadId);
        do
        {
            if (!tClient.Connected)
            {
                tClient.Close();
                Thread.CurrentThread.Abort();       // Kill thread.
            }

            if (tClient.Available > 0)
            {
                byte pByte = (byte)tClient.GetStream().ReadByte();
                Console.WriteLine("Client (Thread: {0}): Data {1}", Thread.CurrentThread.ManagedThreadId, pByte);
                tClient.GetStream().WriteByte(pByte);
            }

            // Pause
            Thread.Sleep(100);
        } while (true);
    }
}
Olha Shumeliuk
  • 720
  • 7
  • 14
  • 4
    How do you know that happens? – Camilo Terevinto Oct 15 '18 at 11:36
  • I have a tcp echo server and if I set breakpoint on line after `await stream.WriteAsync(sendData, 0, sendData.Length);` i see that data is comming till I won't hit F5. And then if I continue execution data sending stops. – Olha Shumeliuk Oct 15 '18 at 11:40
  • Who and how is `SendDataAsync` called? Please post the entire call stack – Camilo Terevinto Oct 15 '18 at 11:41
  • @CamiloTerevinto, updated question – Olha Shumeliuk Oct 15 '18 at 11:53
  • it should be there. you could also call `.Flush` on the stream – Daniel A. White Oct 15 '18 at 11:58
  • @DanielA.White, doesn't work either – Olha Shumeliuk Oct 15 '18 at 12:09
  • 1
    So you have placed a breakpoint after the line with the await, that breakpoint was hit, and the target app did not receive the data? Nagling might cause a 200ms delay but not more. This definitely should work. Are you sure the data was not received? – usr Oct 16 '18 at 20:48
  • The write action returns when the data is queued instead of sent, if you dispose it, it might be lost. Normally I use the `socket.Shutdown` to flush it. But it looks like you should use the `Close` of the `TCPClient` – Jeroen van Langen Oct 16 '18 at 20:48
  • @OlgaShumeliuk If your echo server echoes back on the same connection it could be the issue. Can you confirm that the data is not received rather than just not echoed? – pere57 Oct 17 '18 at 19:27
  • @pere57 Apparantly there was some issue wuth echo server, because when I start send data to real app - everything works fine and withing seconds. While with echo server everything worked slow and weren't able to read the whole data. "If your echo server echoes back on the same connection it could be the issue" - yes. I will update my question with the echo server code – Olha Shumeliuk Oct 18 '18 at 05:28
  • @usr, updated question. Maybe issue was in echo server, but not in client side – Olha Shumeliuk Oct 18 '18 at 05:33

3 Answers3

1

The easy part is that the echo server works slowly because it pauses for 100ms after each read. I guess that's so you get a chance to see what's happening.

For why you don't see all the data, I'm not sure exactly, but what I think might be happening is:

  • When your client execution leaves the using block, the stream is disposed (thanks to Craig.Feied in his answer for pointing out that execution proceeds before the underlying sockets have finished the physical transmission of the data)
  • Disposing the NetworkStream causes it to issue a shutdown to the underlying Socket
  • The shutdown gives the chance for the Socket to finish sending any buffered data before it finally closes. Reference: Graceful Shutdown, Linger Options, and Socket Closure
  • Note there is no data buffered by the NetworkStream itself as it passes all writes directly to the socket. So even if the NetworkStream is disposed before the transfer completes no data is lost
  • A socket in shutdown can complete existing requests but won't accept new requests.

So, your echo server receives data from the transfer already in progress (okay) but then issues a new write request on the connection (not okay.) I suspect this write is causing the echo server to exit early without reading all the data. Either:

  • the client closes the connection because it receives data it's not expecting, or
  • the echo server throws an uncaught exception on tClient.GetStream().WriteByte(pByte);

It should be easy to check if it is indeed either of the above.

pere57
  • 652
  • 9
  • 18
1

The echo server is broken. It sleeps after every byte for 100ms. That way it will take a long time to reply to your message.

The Available check is always wrong. No need check before reading and not need to sleep. The check for being connected also does nothing because the client could disconnect right after the check.

I think this should work:

tClient.GetStream().CopyTo(tClient.GetStream());

Everything else can be deleted.

usr
  • 168,620
  • 35
  • 240
  • 369
-1

Your code wraps an async process in a using block, so naturally code execution continues, hits the end of the using block, and disposes first the stream and then the TcpClient -- before the underlying sockets have finished the physical transmission of data. Using and Async aren't always a good match, as you have seen.

Your await signs up the remainder of the method as the continuation for the async operation, and then it returns to the caller immediately. The continuation is executed as soon as WriteAsync returns.

Perhaps you were expecting that WriteAsync would return only after all the bytes had been successfully transmitted. That seems logical but is not correct. WriteAsync returns when it has written data to the stream, but that doesn't mean the data has gone out over TCPIP. The stream will still be in use with socket activity ongoing when WriteAsync returns.

Since your continuation contains nothing, you exit the using block immediately and the stream gets disposed while it's still in use.

So I think there is nothing wrong with your client or your stream, and flushing it won't help. You are getting exactly the behavior expected from the code you've written, given the behavior of your Echo server.

You can read Eric Lippert's blog post about await async here.

You can read another SO question involving a similar problem here.

If you don't control the Echo Server then you basically have two options. You can give up using -- instead just instantiate the client and the stream, then use them over some natural set of circumstances, and only close them when (somewhere later in your application) you are confident you are done with them. Naturally you will need some way to detect that you are done (or get to some point where you want to close them whether or not the data was successfully received somewhere).

OTOH, if you want to retain the using block then you must put some code inside your continuation to test for completion, so you don't close the stream for further communications as soon as WriteAsync has handed off the data to the socket and requested socket shutdown.

If you are sending very small messages and are confident you know how long it will take for them to be sent or become stale, then you could just add a manual await Task.Delay() -- but that's a dirty approach that will eventually cause problems.

Or you could use a method that is not asynchronous. The async methods are async all the way down, so WriteAsync will call or implement async methods like Stream.BeginWrite, which of course returns before transmission is complete. The Non-async methods, such as Write() will call or implement non-async stream methods like Stream.Write() -- which return only after the byte has actually been sent by the socket. Absent network problems, this most often means your data will be received even if you aren't doing acknowledgement at the application level. See the Framework for streams.

The Write method blocks until the requested number of bytes is sent or a SocketException is thrown.

Of course, if you own the Echo server then you can look there to see why it stops showing data when it can no longer communicate with your disposed network stream.

Craig.Feied
  • 2,617
  • 2
  • 16
  • 25
  • I'm not following why the stream goes out of scope before the `WriteAsync` task has completed, given the use of `await`. If there was code within the using block and after the awaited statement, wouldn't it be able to reference `stream`? – pere57 Oct 15 '18 at 22:01
  • @pere57 This pitfall bites people all the time. Await doesn't block the current thread until the async operation returns -- it does the opposite: it signs up the remainder of the method (i.e. nothing) as the continuation of the task and returns to the caller immediately. Eric Lippert blogged about this here: https://blogs.msdn.microsoft.com/ericlippert/2010/10/29/asynchronous-programming-in-c-5-0-part-two-whence-await/. If any logic after the await but within the `using` were to block until completion, it would certainly have access to the stream. – Craig.Feied Oct 16 '18 at 20:16
  • 1
    I am not sure this is correct. I believe the `using` block is syntactic sugar for a `finally` clause that is part of the continuation, and will not execute until the awaitable has signaled completion (assuming you use `await`, which OP does). You are correct about the pitfall, but that is when forgetting to `await`. – John Wu Oct 16 '18 at 20:24
  • 1
    https://stackoverflow.com/questions/16566547/do-using-statements-and-await-keywords-play-nicely-in-c-sharp – pm100 Oct 16 '18 at 20:32
  • @JohnWu I agree with what you've written, but WriteAsync does not wait until bytes have physically left the device before returning. It returns when it has passed off the data to its underlying layers, which likely include buffering at least at the socket level. It may return before all data has been sent, and because of the using block (with nothing in the continuation) the stream will be destroyed at that point. – Craig.Feied Oct 16 '18 at 20:38
  • 1
    Agreed. I don't think of that as having anything to do with async/await, though. It has to do with writebehind caching. You'd have the same issue if the call were fully synchronous. If that is the issue, the solution would be to `Flush()` the socket, not to monkey with the using block. – John Wu Oct 16 '18 at 20:58
  • @JohnWu Flush doesn't help because WriteAsync returns *as soon as it has passed the data to the stream*. The stream could be blocked on communications handshaking or anything else. It will still be disposed as soon as WriteAsync returns. Of course you can still close a stream in mid-transfer even if the call were fully synchronous. My only real point is that you want to test for completion before you allow the stream to be disposed. Neither 'await' nor 'WriteAsync` make that happen, and the `using` block is why the stream is disposed in the code sample posted :) – Craig.Feied Oct 16 '18 at 21:52
  • You're right. `Flush()` on the stream isn't the right answer. OP has to call `client.Client.ShutDown()` I believe, prior to disposing. – John Wu Oct 16 '18 at 22:04