0

in server -(multi)client application [TCP]. I use Socket, NetworkStream, StreamReader and StreamWriter for each client i Accept in the server .. so i have couple of questions :

  • Do i have to dispose all of them when i want to close the connection with a client?
  • If the client Disposes the Socket that's connected with the server .. do i also have do dispose that socket in the server side ,or it closes automatically ?
  • in my code here :
     Thread thAccept = new Thread(acceptClient);
     Thread thJob;
     private void acceptClient()
        {
            while (true)
            {
                Socket client = server.Accept();
                Console.WriteLine(client.RemoteEndPoint+" has connected");
                StreamReader reader = new StreamReader(new NetworkStream(client));
                //is it ok to create an instance NetworkStream like this or i will have to dispose it later?
    
                thJob = new Thread(Job);
                thJob.Start(reader);
            }
        }
    
     private void Job(object o)
        {
            StreamReader reader = (Socket)o;
            try
            {
                string cmd = null;
                while ((cmd = reader.ReadLine()) != null)
                {
                  //(BLA BLA..)
                }
            }
            catch
            {
                Console.WriteLine("Disconnected by catch");  
            }
            finally
            { 
                Console.WriteLine("Finally Done.");
                reader.Dispose();
            }
        }
    

    is that code fine to dispose all (needed to be disposed) objects?

  • John Saunders
    • 160,644
    • 26
    • 247
    • 397
    Murhaf Sousli
    • 12,622
    • 20
    • 119
    • 185
    • possible duplicate of [Proper use of the IDisposable interface](http://stackoverflow.com/questions/538060/proper-use-of-the-idisposable-interface) – John Saunders Mar 21 '12 at 05:01
    • 1
      Please do a little searching before posting a question. This one has been asked a dozen times or more. – John Saunders Mar 21 '12 at 05:01
    • @JohnSaunders well, im still beginner to c# .. i couldn't understand all of the answer of your link.. but i think it doesn't answer my questions .. its kinda a different question .. i dept that you read it to the end .. you just read the subject! there are sub-questions that's important to me .. please if you don't want to help.. let others to help me and don't close my question. – Murhaf Sousli Mar 21 '12 at 06:39
    • Ok, you're almost right. I read the title _and_ the text of your question. I did not read the code. This is not a duplicate. – John Saunders Mar 21 '12 at 14:52

    1 Answers1

    0

    This is not a duplicate.

    Your code differs from the linked duplicate because in your code, the IDisposable is handed off to another thread.

    The general rule is that if you create an object that implements IDisposable, then you're responsible for calling Dispose on it when you're finished with it. When possible, that should be done in a using block, to ensure that Dispose is always called. In your case, your code is not finished with the object until the other thread is exited. In that thread, you correctly call Dispose in the finally block.

    If you had called Dispose on the NetworkStream, then it would have closed the StreamReader as well, which would defeat your purpose. I suspect it would be the same thing if you had called Dispose on the Socket. As such, your code is correct as-is.

    The object on the client side has no relationship with the object on the server side, except through TCP/IP. The fact that the client may call Dispose on its socket doesn't mean that the server has to call Dispose on its socket. However, once the server is finished reading data from the socket, and the connection is closed, the server-side socket should be Disposed. I don't know for certain, but I believe that when the StreamReader is disposed, the underlying NetworkStream will be disposed, which should call Dispose on the socket from which the stream was created.

    Your code is fine, except for some unrelated issues: you don't need to set cmd to null, since you're going to set the value in the next statement. Also, you should not use an empty catch block like that. You have no idea what exception was thrown, but you will ignore it anyway, without even logging or displaying the exception. At the least, you should do

        catch (Exception ex)
        {
    
            Console.WriteLine("Disconnected by exception " + ex.ToString());   
        } 
        finally 
        {  
            Console.WriteLine("Finally Done."); 
            reader.Dispose(); 
        } 
    
    John Saunders
    • 160,644
    • 26
    • 247
    • 397
    • thanks for the answer ... the code i posted is a draft .. i just put it there to know that the connection closed because of the client. there's no need for it later its a silent server .. user shouldn't notify anything. – Murhaf Sousli Mar 24 '12 at 20:25