c# NetworkStream disposed over tcp/ip

Question

I'm hoping someone can tell me why I'm getting an object disposed exception in my tcpListener/tcpClient code.

In the acceptConnections and connectToServer methods I use the keepalive method to tell me when I get disconnected and it works fine.

However, if I uncomment the for loop for my sendMsg method I will get an ObjectDisposedException on the server and an IOException on the client.

The tcpClient.getStream()'s NetworkStream in SendMsg seems to be the issue but I am unsure why it would get a disposed stream. Do I need 2 threads to work with it?

static void Main(string[] args)
{
    server.Listen();
    server.AcceptConnections();

    client.ConnectToServer();

    //for (int i = 0; i < 5; i++) {
    //    Thread.Sleep(3000);
    //    server.SendMsg("SENT MSG");
    //}

    Console.ReadLine();
}

public async void SendMsg(String message) {
        try {
            NetworkStream networkStream = tcpClient.GetStream();
            using (var writer = new StreamWriter(networkStream)) {
                await writer.WriteLineAsync(message);
                Console.WriteLine("msg sent");
            };
        } catch (Exception e) {
        }

    }

private async void KeepAlive(TcpClient tcpClient) {
        bool clientConnected = true;
        using (NetworkStream networkStream = tcpClient.GetStream())
        using (var reader = new StreamReader(networkStream))
        using (var writer = new StreamWriter(networkStream)) {
            writer.AutoFlush = true;
            char keepalive = '0';
            while (clientConnected) {
                try {
                    await writer.WriteLineAsync(keepalive);
                    string dataFromClient = await reader.ReadLineAsync();
                    Console.WriteLine("Server: " + dataFromClient);
                    Thread.Sleep(500);
                } catch (IOException e){

                } catch(ObjectDisposedException e) {
                    clientConnected = false;
                    clientsConnected--;
                } catch (Exception e){
                }
            }
        }
    }

EDIT: posting my AcceptConnections method as well

public async void AcceptConnections() {
        while (true) {
            while (clientsConnected <= maxConnections) {
                try {
                    tcpClient = await tcpListener.AcceptTcpClientAsync();
                    KeepAlive(tcpClient);
                } catch (Exception e) {
                    Console.WriteLine("TOP EXCEPTION :: " + e);
                }
                clientsConnected++;
                Console.WriteLine("SERVER Clients connected: " + clientsConnected);
            }
        }
    }

you aren't *calling* `KeepAlive` - can we see how it is used? what calls `SendMsg`? note that it is very odd and wrong to have two *separate* writers (or readers) on the same stream (I see writers in `SendMsg` and `KeepAlive`) — Marc Gravell, Jan 04 '18 at 12:56
@MarcGravell, I'll edit it in to my question above but KeepAlive is called in AcceptConnections(). SendMsg is called in the main method, I just commented it out for now. — Ian Gleeson, Jan 04 '18 at 16:38

score 1 · Accepted Answer · answered Jan 04 '18 at 12:58

1

Your SendMsg method uses using on a StreamWriter. The default for a StreamWriter is to cascade the dispose, so this will close the NetworkStream. If that isn't your intent, you need to pass leaveOpen: true to the constructor overload.

Frankly though, there's no reason to use StreamWriter here - I would suggest dealing with the Stream and Encoding APIs directly. One advantage of StreamWriter is that internally it might re-use a buffer for the byte[] work, but that "advantage" is moot if you're only using it for one Write before disposing it, and can be readily achieved with a buffer pool.

answered Jan 04 '18 at 12:58

Marc Gravell

1,026,079
266
2,566
2,900

Thanks, I'll try that constructor. I want to reuse the stream reader and writer for the keepalive ping and have a separate one for sending my own messages. Would it be better to use the constructor / only 1 reader/writer / or use the Stream/Encoding API, do you think? – Ian Gleeson Jan 04 '18 at 16:51
@IanGleeson you use whatever your happy with. I'll just say: I deal with networking and IO code ***all the time*** (that's just the area I tend to work in): there is no even remotely conceivable scenario in which I would use `StreamReader` or `StreamWriter` here. – Marc Gravell Jan 05 '18 at 01:02
Thank you for the help, I have a lot to consider. – Ian Gleeson Jan 05 '18 at 11:44

c# NetworkStream disposed over tcp/ip

1 Answers1