TcpClient and StreamReader blocks on Read

Question

Here's my situation:

I'm writing a chat client to connect to a chat server. I create the connection using a TcpClient and get a NetworkStream object from it. I use a StreamReader and StreamWriter to read and write data back and forth.

Here's what my read looks like:

public string Read()
{
 StringBuilder sb = new StringBuilder();
 try
 {
  int tmp;
  while (true)
  {                
   tmp = StreamReader.Read();
   if (tmp == 0)
    break;
   else
    sb.Append((char)tmp);
   Thread.Sleep(1);
  }
 }
 catch (Exception ex)
 {
  // log exception
 }
 return sb.ToString();
}

That works fine and dandy. In my main program I create a thread that continually calls this Read method to see if there is data. An example is below.

private void Listen()
{
 try
 {
  while (IsShuttingDown == false)
  {
   string data = Read();
   if (!string.IsNullOrEmpty(data))
   {
    // do stuff
   }
  }
 }
 catch (ThreadInterruptedException ex)
 {
  // log it
 }
}

...

Thread listenThread = new Thread(new ThreadStart(Listen));
listenThread.Start();

This works just fine. The problem comes when I want to shut down the application. I receive a shut down command from the UI, and tell the listening thread to stop listening (that is, stop calling this read function). I call Join and wait for this child thread to stop running. Like so:

// tell the thread to stop listening and wait for a sec
IsShuttingDown = true;            
Thread.Sleep(TimeSpan.FromSeconds(1.00));

// if we've reach here and the thread is still alive
// interrupt it and tell it to quit
if (listenThread.IsAlive)
    listenThread.Interrupt();

// wait until thread is done
listenThread.Join();

The problem is it never stops running! I stepped into the code and the listening thread is blocking because the Read() method is blocking. Read() just sits there and doesn't return. Hence, the thread never gets a chance to sleep that 1 millisecond and then get interrupted.

I'm sure if I let it sit long enough I'd get another packet and get a chance for the thread to sleep (if it's an active chatroom or a get a ping from the server). But I don't want to depend on that. If the user says shut down I want to shut it down!!

One alternative I found is to use the DataAvailable method of NetworkStream so that I could check it before I called StreamReader.Read(). This didn't work because it was undependable and I lost data when reading from packets from the server. (Because of that I wasn't able to login correctly, etc, etc)

Any ideas on how to shutdown this thread gracefully? I'd hate to call Abort() on the listening thread...

Answer 1

Are you actually using System.IO.StreamReader and System.IO.StreamWriter to send and receive data from the socket? I wasn't aware this was possible. I've only ever used the Read() and Write() methods on the NetworkStream object returned by the TcpClient's GetStream() method.

Assuming this is possible, StreamReader returns -1 when the end of the stream is reached, not 0. So it looks to me like your Read() method is in an infinite loop.

Answer 2

Really the only answer is to stop using Read and switch to using asynchronous operations (i.e. BeginRead). This is a harder model to work with, but means no thread is blocked (and you don't need to dedicate a thread—a very expensive resource—to each client even if the client is not sending any data).

By the way, using Thread.Sleep in concurrent code is a bad smell (in the Refactoring sense), it usually indicates deeper problems (in this case, should be doing asynchronous, non-blocking, operations).