socket with thread closing

Question

I'm creating a client/server application managed by Sockets and Thread. I want to know how to close properly the socket and the related threads. This is my code:

protected override void OnFormClosing(FormClosingEventArgs e)
{
    base.OnFormClosing(e);

    try
    {
        if (UserList.Count > 0)
        {
            foreach (User user in UserList)
            {
                Socket socketUser = user.getSocket();
                socketUser.Send(Encoding.ASCII.GetBytes("!close"));
                socketUser.Close();
            }
        }
        thrAccept.Abort();
        thrReceive.Abort();
        socket.Close();
        incoming.Close();

        Application.Exit();
    }
    catch
    {
        MessageBox.Show("aasa");
    }
}

private void connectBtn_Click(object sender, EventArgs e)
{
    string port=portNum.Text;
    string welcome = "Server up and running - " + System.DateTime.Now.GetDateTimeFormats()[0] + "  " + DateTime.Now.ToString("HH:mm") + "\n" + "Listening Port: " + port + "\n";
    socket.Bind(new IPEndPoint(IPAddress.Any, Convert.ToInt32(portNum.Text)));
    socket.Listen(3);

    thrAccept = new Thread(new ThreadStart(Accept));
    serverLog.AppendText(welcome + "\n");
    thrAccept.Start();
    connectBtn.Enabled = false;
}

private void Accept()
{
    while (true)
    {
        incoming = socket.Accept();
        thrReceive = new Thread(new ThreadStart(Receive));
        thrReceive.Start();
    }
}

private void Receive()
{
    User user = new User();//create an istance for the class User

    while (true)
    {
        byte[] buffer = new byte[64];
        incoming.Receive(buffer);
        // the program continue....

How can I close correctly the socket?

Class User

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Net.Sockets;
using System.Threading;

namespace HW2_SERVER
{
    class User
    {
        public string username;
        public Socket S;
        public string role;
        public bool managed;
        public User()
        {
            username = "";
            S = null;
            role = "";
            managed = false;

        }

        public void setManage(bool i)
        {
            managed = i;
        }
        public void setUsername(string u)
        {
            username = u;
        }
        public void setSocket(Socket sock)
        {
            S = sock;
        }
        public void setRole(string r)
        {
            role = r;
        }
        public bool getManaged()
        {
            return managed;
        }
        public string getUsername()
        {
            return username;

        }
        public Socket getSocket()
        {
            return S;
        }
        public string getRole()
        {
            return role;
        }
    }
}

have you thought about calling socket.Shutdown(SocketShutdown.Both); before calling socket.Close? — MethodMan
I try your hint but also like this I riceve the exception (in this case the messagebox) — Francesco
look at the example where you are calling socket.Close, you should also put that in the foreach loop and remove the socket.Close(); call outside of the foreach loop — MethodMan
you may also want to put the code in this event or at least check for it then close it in this Event OnSocketConnected — MethodMan
yes what u said is right, but actually i'm trying to close the socket without clients connected, so that if will be executed only in case there's at least 1 user connected to the server, so now i don't need to use socket.close() in that loop — Francesco

Drew Noakes Drew Noakes · Accepted Answer · 2012-12-30T18:54:40

You seem to be overwriting your reference to socket and incoming. I assume these are fields on the class. I think you need to keep a collection of these objects, and close all of them when you're shutting down.

Also, aborting threads is a bad idea as you don't know what operation the thread was doing at the time it was being aborted. It's considered a better practice to make a graceful shutdown by setting a flag and checking it in the loop. This may be the cause of your exception. I suggest you don't abort the threads, and instead find a proper way to shut everything down. You're more likely to find a bug-free path this way.

I would replace both of your while (true) loops with while (_isRunning). Set that as a field on the class with a public value. When you're closing down, set it to false and the threads will exit gracefully.

socket with thread closing

1 Answers