What is the proper way to implement a line based network worker class

Question

I'm building an IRC client, and want an IRC independant networking class that just sends and receives lines between the client and server. Sending lines is OK, but I'm at a loss for a good way of implementing a listener thread. All the methods I've tried work, but have flaws that I can't get around.

My first method was to create a StreamReader called _reader using TcpClient.GetStream() as a base, and just doing a _reader.ReadLine(). This has the advantage that the background listener thread automatically stops until a line is received. The problem is that when I disconnect and stop the listener thread (or the application just exits), I'm not sure how good it is for garbage collection etc that the thread with the waiting ReadLine() just gets killed. Could the thread get orphaned and somehow stay in the background stealing resources?

My second method was to create a NetworkStream called _reader based on the same TcpClient.GetStream(), and create a loop that checks _reader.DataAvailable and continue the while loop if it's false, and otherwise push the bytes into a StringBuilder that is instantly checked for \r\n and extracts any whole lines. This gives the same effect as the ReadLine on data retrieval, but I don't like the constant _reader.DataAvailable looping. On my Core i5 CPU, it doesn't take any CPU but on my much more powerful i9 laptop it steals a virtual CPU constantly. Thread.Sleep(1) "solves" this, but seems like a dirty fix, and numerous articles around the internet classify this as a code smell.

So what would be the proper solution?

score 1 · Answer 1 · answered Apr 27 '11 at 16:56

1

If you're aborting the listener thread, then that's bad. But if the application exits (or if you close the underlying socket), then it'll be just fine.

List most others, I don't recommend polling. I would suggest a solution that uses asynchronous reads and writes, though it is much more difficult to program.

answered Apr 27 '11 at 16:56

Stephen Cleary

437,863
77
675
810

I've looked at the asynchronous methods, but the MSDN documentation - and especially the example code - is full of flaws and errors. I still haven't found a good guide on how to use them. – carlsb3rg Apr 27 '11 at 18:43
True. The most useful resources are for unmanaged TCP/IP applications. I have a few guidelines in section 2 of my [FAQ](http://nitoprograms.blogspot.com/2009/04/tcpip-net-sockets-faq.html), but it's not tutorial-level material. – Stephen Cleary Apr 27 '11 at 18:57
I never abort the thread. It's a background thread that dies automagically when the application closes. The listen loop is `while (tcpClient.Connected) {...}`. If I use `.ReadLine()` then there's no way of forcing program execution to continue to the next `while` check and end the loop gracefully. That's why I want to use the NetworkStream method since I can loop continuously while data is not available and exit gracefully when needed. I liked your FAQ thugh. You should get around to the bits that are for a futre FAQ entry. – carlsb3rg Apr 27 '11 at 20:57
I just realized that I shouldn't be using `TcpClient.Connected` for my `while` condition since it will never change unless the loop actually reads data. I shouldn't even be using `DataAvailable` according to [this](http://social.msdn.microsoft.com/Forums/en/netfxnetcom/thread/d1080a84-4cb1-4075-8eb7-3a4c7a194cf6) post :) – carlsb3rg Apr 27 '11 at 21:02
1

Finishing that FAQ *is* on my todo list. Along with many other things. ;) – Stephen Cleary Apr 28 '11 at 03:16
I've found another IRC library and looked at the source code, and they use `Socket` and `ReadAsync` and `WriteAsync` instead. Maybe they're better for long running bi-directional TCP communication? – carlsb3rg Apr 28 '11 at 19:07
`*Async` are newer async APIs, added to avoid the allocation overhead implicit with `Begin*/End*`. I wrote some socket wrappers as part of [Nito.Async](http://nitoasync.codeplex.com/), but they use `Begin*/End*` for two reasons: 1) The performance difference is unnoticeable except for a tiny percentage of applications, and 2) You really need to know specific application usage scenarios to actually get the performance boost; a general-purpose library like mine would be hard-pressed to strike a balance between usability and performance if it was built on `*Async`. – Stephen Cleary Apr 28 '11 at 19:29

score 0 · Answer 2 · edited May 23 '17 at 11:43

Old question, the issue must have settled long ago. Anyway, it looks very much like the even older question c# - What is a good method to handle line based network I/O streams? - Stack Overflow

If so, the answer there should provide part of the solution. Basically, it's a class that provides a Process(byte[]) method returning an IEnumerable<string>, which keeps state of the partial content not yet forming a full line.

Using that will allow to separate concerns. Just read data as it is available. Every time a chunk is read, feed it into that method, you get lines as they are fully formed.

score 0 · Answer 3 · answered Apr 29 '11 at 13:09

Here is something to get you going.

Note that it's not very efficient to use string when building the messages. But it makes everything much more readable.

public abstract class LineBasedChannel
{   
    Socket _socket;
    string _inbuffer = string.Empty;
    Encoding _encoding;
    byte[] _buffer = new byte[8192];

    public LineBasedChannel(Socket socket)
    {
        _socket = socket;
        _encoding = Encoding.ASCII;
        _sb = new StringBuilder();
    }

    public void Start()
    {
        _socket.BeginReceive(_buffer, 0, _buffer.Length, SocketFlags.None,OnRead, null);
    }

    public void OnRead(IAsyncResult res)
    {
        var bytesRead = _socket.EndReceive(res);
        if (bytesRead == 0)
        {
            HandleDisconnect();
            return;
        }

        _inbuffer += _encoding.GetString(_buffer, 0, bytesRead);
        _socket.BeginReceive(_buffer, 0, _buffer.Length, SocketFlags.None,OnRead, null);

        while (true)
        {
            int pos = _inbuffer.IndexOf("\r\n");
            if (pos == -1)
                break;

            OnReceivedLine(_inbuffer.SubString(0, pos+2);
            _inbuffer = _inbuffer.Remove(0,pos+1);
        }
    }

    protected abstract void OnReceivedLine(string line);
}

public class IrcTcpChannel : LineBasedChannel
{
    protected override void OnReceivedLine(string line)
    {
        var cmd = new IrcCommand();
        cmd.Channel = line.SubString(x,y);
        cmd.Message = line.SubString(z,a);
        CommandReceived(this, new IrcCommandEventArgs(cmd));
    }

    public event EventHandler<IrcCommandEventArgs> CommandReceived = delegate {};
}

Also note that I didn't handle any exceptions in that code, which is mandatory. Any unhandled exceptions in async methods will terminate your application.

What is the proper way to implement a line based network worker class

3 Answers3