Jeremy Morren
Jeremy Morren

Reputation: 764

TcpListener + TcpClient - wait for client to read data before closing

I am building a simple HTTP server for PDF files with TcpClient. It works well, however the TcpClient closes before the browser is downloading of the PDF is being finished. How can I force TcpClient to wait until the remote client get everything that is written before closing?

//pdf is byte[]

TcpListener server = new TcpListener(address, port);

server.Start();

TcpClient client = server.AcceptTcpClient();  //Wait for connection

var ns = client.GetStream();

string headers;

using (var writer = new StringWriter())
{
    writer.WriteLine("HTTP/1.1 200 OK");
    //writer.WriteLine("Accept:  text/html");
    writer.WriteLine("Content-type:  application/pdf");
    writer.WriteLine("Content-length: " + pdf.Length);
    writer.WriteLine();
    headers = writer.ToString();
}

var bytes = Encoding.UTF8.GetBytes(headers);
ns.Write(bytes, 0, bytes.Length);

ns.Write(pdf, 0, pdf.Length);

Thread.Sleep(TimeSpan.FromSeconds(10)); //Adding this line fixes the problem....

client.Close();

server.Stop();

Can I replace that ugly 'Thread.Sleep' hack?

EDIT: The code below works, based on the answers:

TcpListener Server = null;

public void StartServer()
{
    Server = new TcpListener(IPAddress.Any, Port);

    Server.Start();

    Server.BeginAcceptTcpClient(AcceptClientCallback, null);
}

void AcceptClientCallback(IAsyncResult result)
{
    var client = Server.EndAcceptTcpClient(result);

    var ns = client.GetStream();

    string headers;

    byte[] pdf = //get pdf

    using (var writer = new StringWriter())
    {
        writer.WriteLine("HTTP/1.1 200 OK");
        //writer.WriteLine("Accept:  text/html");
        writer.WriteLine("Content-type:  application/pdf");
        writer.WriteLine("Content-length: " + pdf.Length);
        writer.WriteLine();
        headers = writer.ToString();
    }

    var bytes = Encoding.UTF8.GetBytes(headers);
    ns.Write(bytes, 0, bytes.Length);

    ns.Write(pdf, 0, pdf.Length);

    client.Client.Shutdown(SocketShutdown.Send);

    byte[] buffer = new byte[1024];
    int byteCount;
    while ((byteCount = ns.Read(buffer, 0, buffer.Length)) > 0)
    {

    }

    client.Close();

    Server.Stop();
}

Upvotes: 1

Views: 1641

Answers (1)

Peter Duniho
Peter Duniho

Reputation: 70701

The main issue in your code is that your server (the file host) neglected to read from the socket it's writing the file to, and so has no way to detect, never mind wait for, the client shutting down the connection.

The code could be way better, but at a minimum you could probably get it to work by adding something like this just before your client.Close(); statement:

// Indicate the end of the bytes being sent
ns.Socket.Shutdown(SocketShutdown.Send);

// arbitrarily-sized buffer...most likely nothing will  ever be written to it
byte[] buffer = new byte[4096];
int byteCount;

while ((byteCount = ns.Read(buffer, 0, buffer.Length)) > 0)
{
    // ignore any data read here
}

When an endpoint initiates a graceful closure (e.g. by calling Socket.Shutdown(SocketShutdown.Send);), that will allow the network layer to identify the end of the stream of data. Once the other endpoint has read all of the remaining bytes that the remote endpoint has sent, the next read operation will complete with a byte length of zero. That's that other endpoint's signal that the end-of-stream has been reached, and that it's time to close the connection.

Either endpoint can initiate the graceful closure with the "send" shutdown reason. The other endpoint can acknowledge it once it's finished sending whatever it wants to send by using the "both" shutdown reason, at which point both endpoints can close their sockets (or streams or listeners or whatever other higher-level abstraction they might be using the wrap the socket).

Of course, in a properly-implemented protocol, you'd know in advance whether any data would ever actually be sent by the remote endpoint. If you know none ever will be, you could get away with a zero-length buffer, and if you know some data might be sent back from the client, then you'd actually do something with that data (as opposed to the empty loop body above).

In any case, the above is strictly a kludge, to get the already-kludged code you posted to work. Please don't mistake it for something intended to be seen in production-quality code.

All that said, the code you posted is a long way from being all that good. You aren't just implementing a basic TCP connection, but apparently are trying to reimplement the HTTP protocol. There's no point in doing that, as .NET already has HTTP server functionality built in (see e.g. System.Net.HttpListener). If you do intend to reinvent the HTTP server, you need a lot more code than the code you posted. The lack of error-handling alone is a major flaw, and will cause all kinds of headaches.

If you intend to write low-level network code, you should do a lot more research and experimentation. One very good resource is the Winsock Programmer’s FAQ. It's primary focus is, of course, programmers targeting the Winsock API. But there is a wealth of general-purpose information there as well, and in any case all of the various socket APIs are very similar, as they are all based on the same low-level concepts.

You may also want to review various existing Stack Overflow Q&A. Here are a couple of ones closely related to your specific problem:
How to correctly use TPL with TcpClient?
Send a large file over tcp connection

Do be careful though. There's almost as much bad advice out there as good. There's no shortage of people going around acting like they are experts in network programming when they aren't, so take everything you read with a grain of salt (including my advice above!).

Upvotes: 1

Related Questions