5

I'm creating a server that a TCP connection. The TCP Connection is run in its own thread for an indefinite amount of time. Is there a good pattern to allow safe shutdown of the TcpListener and Client as well as the thread? Below is what I have so far.

private volatile bool Shudown;

void ThreadStart1()
{
    TcpListener listener = null;
    TcpClient client = null;
    Stream s = null;
    try
    {
        listener = new TcpListener(60000);
        client = listener.AcceptTcpClient();
        Stream s = client.GetStrea();
        while(!Shutdown)  // use shutdown to gracefully shutdown thread.
        {
            try
            {
                string msg = s.ReadLine();  // This blocks the thread so setting shutdown = true will never occur unless a client sends a message.
                DoSomething(msg);
            }
            catch(IOException ex){ } // I would like to avoid using Exceptions for flow control
            catch(Exception ex) { throw; }
        }
    }
    catch(Exception ex)
    {
        LogException(ex);
        throw ex;
    }
    finally
    {
        if(listener != null) listener.Close();
        if(s != null) s.Close();
        if(client != null) client.Close();
    }
}
Neal
  • 599
  • 8
  • 16
  • Your catch clause is redundant. It's not a solution to your question, but it's still redundant. – zmbq Nov 27 '11 at 06:13
  • Your right. I edited to show my true intention. – Neal Nov 27 '11 at 06:14
  • 3
    On a similar note, you shouldn't do `throw ex` as that resets your stack trace. Try using just `throw` instead. See: http://stackoverflow.com/questions/178456/what-is-the-proper-way-to-re-throw-an-exception-in-c – Kiley Naro Nov 27 '11 at 06:18
  • Correct me if I'm wrong, but is never a good idea to abort a thread while it is running (even though it is possible). A better suggestion would be to rework your code to read a variable, if the variable is set to true, the thread can break out of any loops/waits and finish executing, so it isn't necessarily to abort it. – Jess Nov 27 '11 at 06:19
  • @mazzzz: How would you suggest solving the question? I realize aborting the thread is bad. That is why I'm asking for a graceful way to handle killing the thread while also killing the tcp connection gracefully. – Neal Nov 27 '11 at 06:21
  • @mazzz: I agree that is a normal thread execution pattern. However, in this instance, the thread is blocked until a message is received. I have a choice to implement a timeout on the connection, however, that will throw an exception. – Neal Nov 27 '11 at 06:23

3 Answers3

5

Set a timeout on the NetworkStream (client.ReadTimeout=...). Once the read operation times out, check to see if the main thread signalled you to stop (by setting a variable or an AutoResetEvent). If it's been signalled to stop, exit gracefully. If not, try reading again until the next timeout.

Setting a 0.5 or 1 second timeout should suffice - you will be able to exit the thread in a timely manner, and yet be very easy on the CPU.

zmbq
  • 38,013
  • 14
  • 101
  • 171
  • Beat me to it, and this would be my solution as well, just catch the exception make sure it was a timeout error (if it was something else throw it) and continue the loop if the variable is still false – Jess Nov 27 '11 at 06:26
  • That will throw an exception. How do I handle that without loosing the connection? It would be nice if there was a timeout option that did not throw a connection but instead returned a null or somesuch to allow unblocking a thread at a given interval. – Neal Nov 27 '11 at 06:29
  • The connection isn't close when the exception is closed. Catch the IOException, make sure it's a timeout (look at the HRESULT) and just keep reading in a loop. – zmbq Nov 27 '11 at 06:38
  • Due to exceptions being fairly expensive, I would prefer to avoid them for flow control. The Tcp connection needs to be fairly responsive (~1-2ms). In general, Exceptions should not be used for flow control. – Neal Nov 27 '11 at 06:48
  • 1
    You're takling about an exception after a second if inactivity. I think it's neglible. If it isn't, drop the background thread altogether and switch to asynchronous IO (use BeginRead instead of Read and supply a callback) – zmbq Nov 27 '11 at 06:58
  • @zmbq: The connection could be in a wait state for quite a while. (30 mins between messages). The server is basically a remote connection to local hardware where a command is sent to the server which is forwarded to the hardware. We need the commands to have very low latency. Catching an exception could mean 3-4ms lost. Thanks for the asynchronous IO suggestion however, I'll look into that to see if it will work for what I need. – Neal Nov 27 '11 at 07:36
  • Another option is not to worry about killing the thread in case your process finishes. The OS will shut down the connection for you. Use the same read as you do now. Nobody is really going to notice the unclean cleanup. – zmbq Nov 27 '11 at 18:31
  • @zmbq: I think Hans and GETah got it right in thier comments below. Forcing the OS to do the cleanup does not sound desireable at all. – Neal Nov 28 '11 at 15:03
2

Is there a good pattern to allow safe shutdown of the thread?

Change the while loop to the following:

while (!interrupted){
   // Do something
}
// declare interrupted as volatile boolean
volatile bool interrupted;

Check this MSDN example for details. Setting interrupted boolean to true will make the thread come out of the loop when it checks for the while condition.

Is there a good pattern to allow safe shutdown of the TcpListener and Client?

To avoid duplication, please check this SO question

As for your question on how to terminate a blocking thread on ReadLine(); the following listener.Server.Close(); should do the job and return from the blocking call.

Community
  • 1
  • 1
GETah
  • 20,922
  • 7
  • 61
  • 103
  • That is a comment pattern for safe thread shutdown. But that does not solve the question. How do I tell the thread it is interrupted if it is blocked by the Stream.ReadLine()? I'll update the question to show what I'm talking about. – Neal Nov 27 '11 at 16:09
  • 1
    listener.Server.Close() from another thread will break the ReadLine blocking call. – GETah Nov 27 '11 at 16:28
  • Really? I didn't think about trying that. I'll see if that works. If that works for what I need, you can create an answer and I'll accept it as the solution. – Neal Nov 27 '11 at 17:03
  • 3
    Using Server.Close() is the proper answer. Any blocking method will complete with a ObjectDisposedException. – Hans Passant Nov 27 '11 at 17:09
1

Perhaps instead of calling Read on a NetworkStream object synchronously, you should use BeginRead and EndRead to do it asynchronously, and call Close() on the NetworkStream when you are done with it.

bbosak
  • 5,353
  • 7
  • 42
  • 60