C# многопоточный чат-сервер, дескриптор отключения

Каждый раз, когда я закрываю клиент, сервер перестает работать. Я получаю сообщение об ошибке, что он "не может читать дальше конца потока"

Это, в некотором смысле, совершенно нормально. То есть при использовании BinaryReader его нормальным поведением является бросание EndOfStreamException при достижении конца потока. Почему он достиг конца потока? Ну, потому что клиент отключился, и это то, что происходит с потоком. На уровне сокета то, что действительно происходит, - это чтение операция завершается с 0 как количество прочитанных байт. Это указывает на то, что клиент изящно закрыл сокет и больше не будет отправлять данные.

В .NET API это переводится в конец NetworkStream, который TcpClient использует для обертывания объекта Socket, который фактически обрабатывает сетевой ввод-вывод, и этот объект NetworkStream, в свою очередь, обертывается вашим объектом BinaryReader. И BinaryReader выбрасывает это исключение, когда оно достигает конца потока.

Обратите внимание, что ваш код на самом деле это не дает пользователю изящного способа закрыть клиент. Они должны будут использовать Ctrl+C , или убить процесс сразу. Использование первого имеет случайный эффект выполнения изящного завершения работы сокета, но только потому, что .NET обрабатывает завершение процесса и запускает завершители на ваших объектах, таких как объект TcpClient, используемый для подключения к серверу, и завершитель вызывает Socket.Shutdown(), чтобы сообщить серверу, что он закрывается.

Если вы если бы вы убили процесс (например, с помощью Диспетчера задач), вы бы обнаружили, что вместо него был брошен IOException. Хороший сетевой код всегда должен быть готов увидеть IOException; сети ненадежны, и сбои действительно происходят. Вы хотите сделать некоторые разумные действия, такие как удаление удаленной конечной точки из ваших соединений, а не просто сбой всей программы.

Итак, все сказанное только потому, что EndOfStreamException является "нормальным", это не означает, что код, который вы разместили, является или каким-либо образом является примером правильного способ сделать Сетевое программирование. У вас есть ряд проблем:

1. отсутствует четкое корректное закрытие.
   
   Сетевой ввод-вывод обеспечивает нормальный способ закрытия соединений, который включает в себя рукопожатие на обеих оконечных точках, чтобы указать, когда они закончили отправку, и когда они закончили прием. Одна конечная точка укажет, что она отправлена; другая отметит это (используя 0-байтовое чтение, упомянутое выше), а затем сама укажет, что она отправлена и получена.
   
   TcpClient и NetworkStream не выставляйте это напрямую, но вы можете использовать свойство TcpClient.Client, чтобы заставить объект Socket сделать более изящное закрытие, то есть одна конечная точка может указать, что она закончила отправку, и все еще быть в состоянии ждать, пока другая конечная точка также не закончит отправку.
   
   Использование метода TcpClient.Close() для отключения-это все равно что повесить трубку на кого-то, не сказав "До свидания". Использование Socket.Shutdown() похоже на завершение телефонного разговора вежливым "хорошо, это все, что я хотел сказать...было ли что-нибудь еще что-нибудь?"
2. вы используете BinaryReader, но не обрабатываете EndOfStreamException правильно.
Ваш клиент использует два соединения для связи с сервером.

Сетевой ввод-вывод использует объект Socket, который поддерживает полнодуплексную связь. Нет необходимости создавать второе соединение только для чтения и записи. Достаточно одного соединения, и это лучше, потому что когда вы разделяете отправку и получение на два соединения, то вам также нужно добавить что-то к вашему протоколу, так что что сервер знает, что эти два соединения представляют собой одного клиента (что ваш код делает не на самом деле).
3. клиент не удаляется из списка серверов при его отключении (вы отметили это в своем вопросе).
4. список клиентов не является потокобезопасным.
5. в вашем методе Chat() есть дополнительное "while (true)".

Я изменил ваш исходный пример, чтобы обратиться ко всем вышеперечисленным, которые я представил здесь:

Сервер Программа.cs:

class Program
{
    private static readonly object _lock = new object();
    private static readonly List<TcpClient> clients = new List<TcpClient>();

    public static TcpClient[] GetClients()
    {
        lock (_lock) return clients.ToArray();
    }

    public static int GetClientCount()
    {
        lock (_lock) return clients.Count;
    }

    public static void RemoveClient(TcpClient client)
    {
        lock (_lock) clients.Remove(client);
    }

    static void Main(string[] args)
    {
        IPAddress ip = IPAddress.Parse("127.0.0.1");
        TcpListener ServerSocket = new TcpListener(ip, 14000);
        ServerSocket.Start();

        Console.WriteLine("Server started.");
        while (true)
        {
            TcpClient clientSocket = ServerSocket.AcceptTcpClient();
            Console.WriteLine($"client connected: {clientSocket.Client.RemoteEndPoint}");
            lock (_lock) clients.Add(clientSocket);
            handleClient client = new handleClient();
            client.startClient(clientSocket);

            Console.WriteLine($"{GetClientCount()} clients connected");
        }
    }
}

Сервер handleClient.cs:

public class handleClient
{
    TcpClient clientSocket;

    public void startClient(TcpClient inClientSocket)
    {
        this.clientSocket = inClientSocket;
        Thread ctThread = new Thread(Chat);
        ctThread.Start();
    }

    private void Chat()
    {
        BinaryReader reader = new BinaryReader(clientSocket.GetStream());

        try
        {
            while (true)
            {
                string message = reader.ReadString();
                foreach (var client in Program.GetClients())
                {
                    BinaryWriter writer = new BinaryWriter(client.GetStream());
                    writer.Write(message);
                }
            }
        }
        catch (EndOfStreamException)
        {
            Console.WriteLine($"client disconnecting: {clientSocket.Client.RemoteEndPoint}");
            clientSocket.Client.Shutdown(SocketShutdown.Both);
        }
        catch (IOException e)
        {
            Console.WriteLine($"IOException reading from {clientSocket.Client.RemoteEndPoint}: {e.Message}");
        }

        clientSocket.Close();
        Program.RemoveClient(clientSocket);
        Console.WriteLine($"{Program.GetClientCount()} clients connected");
    }
}

Клиентская Программа.cs:

class Program
{
    private static readonly object _lock = new object();
    private static bool _closed;

    public static void Write(TcpClient client)
    {
        try
        {
            string str;
            SocketShutdown reason = SocketShutdown.Send;

            while ((str = Console.ReadLine()) != "")
            {
                lock (_lock)
                {
                    BinaryWriter writer = new BinaryWriter(client.GetStream());
                    writer.Write(str);

                    if (_closed)
                    {
                        // Remote endpoint already said they are done sending,
                        // so we're done with both sending and receiving.
                        reason = SocketShutdown.Both;
                        break;
                    }
                }
            }

            client.Client.Shutdown(reason);
        }
        catch (IOException e)
        {
            Console.WriteLine($"IOException writing to socket: {e.Message}");
        }
    }

    public static void Read(TcpClient client)
    {
        try
        {
            while (true)
            {
                try
                {
                    BinaryReader reader = new BinaryReader(client.GetStream());
                    Console.WriteLine(reader.ReadString());
                }
                catch (EndOfStreamException)
                {
                    lock (_lock)
                    {
                        _closed = true;
                        return;
                    }
                }
            }
        }
        catch (IOException e)
        {
            Console.WriteLine($"IOException reading from socket: {e.Message}");
        }
    }

    static void Main(string[] args)
    {
        TcpClient client = new TcpClient("127.0.0.1", 14000);
        Thread writeThread = new Thread(() => Write(client));
        Thread readThread = new Thread(() => Read(client));
        writeThread.Start();
        readThread.Start();

        writeThread.Join();
        readThread.Join();

        client.Close();
        Console.WriteLine("client exiting");
    }
}

Обратите внимание, что я по большей части не обращался к непоследовательным и нетрадиционным именам, которые вы использовали в своем коде. Единственное исключение было сделано для переменных потока в клиентском коде, потому что я действительно не люблю прописные локальные переменные, которые точно соответствуют имени типа.

У вас есть и некоторые другие проблемы, которые выше пересмотр вашего кода не рассматривается. К ним относятся:

1. Вы используете BinaryReader. Это во многих отношениях раздражающий класс для использования. Я рекомендую, особенно для сценария чат-сервера, где вы все равно имеете дело только с текстом, переключиться на использование StreamReader/StreamWriter.
2. существует неправильное сцепление/разделение проблем. Ваш класс Program имеет код сервера, и код сервера знает о классе Program. Было бы гораздо лучше инкапсулировать как сервер, так и клиентские реализации в свои собственные классы, отделенные от основной точки входа программы, и для дальнейшего разделения кода сервера верхнего уровня со структурой данных для каждого клиента (используйте C#event, чтобы позволить коду сервера верхнего уровня получать уведомления о важных событиях, таких как необходимость удаления клиента из списка, без того, чтобы структура данных для каждого клиента действительно знала об объекте сервера верхнего уровня, не говоря уже о его списке клиентов).
3. Вы должны предоставить механизм, чтобы изящно выключил сервер.

Обычно я бы сказал, что они выходят за рамки такого ответа, который уже довольно длинный. Я рассмотрел непосредственную проблему в вашем коде, а затем некоторые, и этого номинально достаточно. Тем не менее, я собирался написать обновленную версию примера basic network programming , который я написал несколько лет назад, как своего рода "промежуточный" пример, добавив поддержку нескольких клиентов, асинхронную операцию, и используя новейшие функции C# (например async/await). Поэтому я пошел вперед и потратил на это некоторое время. Я думаю, что в конце концов я опубликую это в своем блоге...это совсем другой проект. В то же время, вот этот код (обратите внимание, что это полностью с нуля example...it это имело больше смысла, чем пытаться переделать код, который у вас был) ...

Большая часть работы grunt для этой реализации находится в одном классе, совместно используемом сервером и клиентом:

/// <summary>
/// Represents a remote end-point for the chat server and clients
/// </summary>
public sealed class ConnectedEndPoint : IDisposable
{
    private readonly object _lock = new object();
    private readonly Socket _socket;
    private readonly StreamReader _reader;
    private readonly StreamWriter _writer;
    private bool _closing;

    /// <summary>
    /// Gets the address of the connected remote end-point
    /// </summary>
    public IPEndPoint RemoteEndPoint { get { return (IPEndPoint)_socket.RemoteEndPoint; } }

    /// <summary>
    /// Gets a <see cref="Task"/> representing the on-going read operation of the connection
    /// </summary>
    public Task ReadTask { get; }

    /// <summary>
    /// Connect to an existing remote end-point (server) and return the
    /// <see cref="ConnectedEndPoint"/> object representing the new connection
    /// </summary>
    /// <param name="remoteEndPoint">The address of the remote end-point to connect to</param>
    /// <param name="readCallback">The callback which will be called when a line of text is read from the newly-created connection</param>
    /// <returns></returns>
    public static ConnectedEndPoint Connect(IPEndPoint remoteEndPoint, Action<ConnectedEndPoint, string> readCallback)
    {
        Socket socket = new Socket(SocketType.Stream, ProtocolType.Tcp);

        socket.Connect(remoteEndPoint);

        return new ConnectedEndPoint(socket, readCallback);
    }

    /// <summary>
    /// Asynchronously accept a new connection from a remote end-point
    /// </summary>
    /// <param name="listener">The listening <see cref="Socket"/> which will accept the connection</param>
    /// <param name="readCallback">The callback which will be called when a line of text is read from the newly-created connection</param>
    /// <returns></returns>
    public static async Task<ConnectedEndPoint> AcceptAsync(Socket listener, Action<ConnectedEndPoint, string> readCallback)
    {
        Socket clientSocket = await Task.Factory.FromAsync(listener.BeginAccept, listener.EndAccept, null);

        return new ConnectedEndPoint(clientSocket, readCallback);
    }

    /// <summary>
    /// Write a line of text to the connection, sending it to the remote end-point
    /// </summary>
    /// <param name="text">The line of text to write</param>
    public void WriteLine(string text)
    {
        lock (_lock)
        {
            if (!_closing)
            {
                _writer.WriteLine(text);
                _writer.Flush();
            }
        }
    }

    /// <summary>
    /// Initiates a graceful closure of the connection
    /// </summary>
    public void Shutdown()
    {
        _Shutdown(SocketShutdown.Send);
    }

    /// <summary>
    /// Implements <see cref="IDisposable.Dispose"/>
    /// </summary>
    public void Dispose()
    {
        _reader.Dispose();
        _writer.Dispose();
        _socket.Close();
    }

    /// <summary>
    /// Constructor. Private -- use one of the factory methods to create new connections.
    /// </summary>
    /// <param name="socket">The <see cref="Socket"/> for the new connection</param>
    /// <param name="readCallback">The callback for reading lines on the new connection</param>
    private ConnectedEndPoint(Socket socket, Action<ConnectedEndPoint, string> readCallback)
    {
        _socket = socket;
        Stream stream = new NetworkStream(_socket);
        _reader = new StreamReader(stream, Encoding.UTF8, false, 1024, true);
        _writer = new StreamWriter(stream, Encoding.UTF8, 1024, true);

        ReadTask = _ConsumeSocketAsync(readCallback);
    }

    private void _Shutdown(SocketShutdown reason)
    {
        lock (_lock)
        {
            if (!_closing)
            {
                _socket.Shutdown(reason);
                _closing = true;
            }
        }
    }

    private async Task _ConsumeSocketAsync(Action<ConnectedEndPoint, string> callback)
    {
        string line;

        while ((line = await _reader.ReadLineAsync()) != null)
        {
            callback(this, line);
        }

        _Shutdown(SocketShutdown.Both);
    }
}

Клиентская программа будет использовать это класс непосредственно. Серверная часть инкапсулирована в другой класс, находящийся в той же DLL с указанным выше:

/// <summary>
/// Event arguments for the <see cref="ChatServer.Status"/> event
/// </summary>
public class StatusEventArgs : EventArgs
{
    /// <summary>
    /// Gets the status text
    /// </summary>
    public string StatusText { get; }

    /// <summary>
    /// Constructor
    /// </summary>
    /// <param name="statusText">The status text</param>
    public StatusEventArgs(string statusText)
    {
        StatusText = statusText;
    }
}

/// <summary>
/// A server implementing a simple line-based chat server
/// </summary>
public class ChatServer
{
    private readonly object _lock = new object();
    private readonly Socket _listener;
    private readonly List<ConnectedEndPoint> _clients = new List<ConnectedEndPoint>();
    private bool _closing;

    /// <summary>
    /// Gets a task representing the listening state of the servdere
    /// </summary>
    public Task ListenTask { get; }

    /// <summary>
    /// Raised when the server has status to report
    /// </summary>
    public event EventHandler<StatusEventArgs> Status;

    /// <summary>
    /// Constructor
    /// </summary>
    /// <param name="port">The port number the server should listen on</param>
    public ChatServer(int port)
    {
        _listener = new Socket(SocketType.Stream, ProtocolType.Tcp);
        _listener.Bind(new IPEndPoint(IPAddress.Any, port));
        _listener.Listen(int.MaxValue);
        ListenTask = _ListenAsync();
    }

    /// <summary>
    /// Initiates a shutdown of the chat server.
    /// </summary>
    /// <remarks>This method closes the listening socket, which will subsequently
    /// cause the listening task to inform any connected clients that the server
    /// is shutting down, and to wait for the connected clients to finish a graceful
    /// closure of their connections.
    /// </remarks>
    public void Shutdown()
    {
        _listener.Close();
    }

    private async Task _ListenAsync()
    {
        try
        {
            while (true)
            {
                ConnectedEndPoint client = await ConnectedEndPoint.AcceptAsync(_listener, _ClientReadLine);

                _AddClient(client);
                _CleanupClientAsync(client);
            }
        }
        catch (ObjectDisposedException)
        {
            _OnStatus("Server's listening socket closed");
        }
        catch (IOException e)
        {
            _OnStatus($"Listening socket IOException: {e.Message}");
        }

        await _CleanupServerAsync();
    }

    private async Task _CleanupServerAsync()
    {
        ConnectedEndPoint[] clients;

        lock (_lock)
        {
            _closing = true;
            clients = _clients.ToArray();
        }

        foreach (ConnectedEndPoint client in clients)
        {
            try
            {
                client.WriteLine("Chat server is shutting down");
            }
            catch (IOException e)
            {
                _OnClientException(client, e.Message);
            }
            client.Shutdown();
        }

        // Clients are expected to participate in graceful closure. If they do,
        // this will complete when all clients have acknowledged the shutdown.
        // In a real-world program, may be a good idea to include a timeout in
        // case of network issues or misbehaving/crashed clients. Implementing
        // the timeout is beyond the scope of this proof-of-concept demo code.
        try
        {
            await Task.WhenAll(clients.Select(c => c.ReadTask));
        }
        catch (AggregateException)
        {
            // Actual exception for each client will have already
            // been reported by _CleanupClientAsync()
        }
    }

    // Top-level "clean-up" method, which will observe and report all exceptions
    // In real-world code, would probably want to simply log any unexpected exceptions
    // to a log file and then exit the process. Here, we just exit after reporting
    // exception info to caller. In either case, there's no need to observe a Task from
    // this method, and async void simplifies the call (no need to receive and then ignore
    // the Task object just to keep the compiler quiet).
    private async void _CleanupClientAsync(ConnectedEndPoint client)
    {
        try
        {
            await client.ReadTask;
        }
        catch (IOException e)
        {
            _OnClientException(client, e.Message);
        }
        catch (Exception e)
        {
            // Unexpected exceptions are programmer-error. They could be anything, and leave
            // the program in an unknown, possibly corrupt state. The only reasonable disposition
            // is to log, then exit.
            //
            // Full stack-trace, because who knows what this exception was. Will need the
            // stack-trace to do any diagnostic work.
            _OnStatus($"Unexpected client connection exception. {e}");
            Environment.Exit(1);
        }
        finally
        {
            _RemoveClient(client);
            client.Dispose();
        }
    }

    private void _ClientReadLine(ConnectedEndPoint readClient, string text)
    {
        _OnStatus($"Client {readClient.RemoteEndPoint}: \"{text}\"");

        lock (_lock)
        {
            if (_closing)
            {
                return;
            }

            text = $"{readClient.RemoteEndPoint}: {text}";

            foreach (ConnectedEndPoint client in _clients.Where(c => c != readClient))
            {
                try
                {
                    client.WriteLine(text);
                }
                catch (IOException e)
                {
                    _OnClientException(client, e.Message);
                }
            }
        }
    }

    private void _AddClient(ConnectedEndPoint client)
    {
        lock (_lock)
        {
            _clients.Add(client);
            _OnStatus($"added client {client.RemoteEndPoint} -- {_clients.Count} clients connected");
        }
    }

    private void _RemoveClient(ConnectedEndPoint client)
    {
        lock (_lock)
        {
            _clients.Remove(client);
            _OnStatus($"removed client {client.RemoteEndPoint} -- {_clients.Count} clients connected");
        }
    }

    private void _OnStatus(string statusText)
    {
        Status?.Invoke(this, new StatusEventArgs(statusText));
    }

    private void _OnClientException(ConnectedEndPoint client, string message)
    {
        _OnStatus($"Client {client.RemoteEndPoint} IOException: {message}");
    }
}

И что это, по большей части, все, что вам нужно. На приведенный выше DLL-код ссылаются (в моем примере) две разные программы, сервер и клиент.

Вот сервер:

class Program
{
    private const int _kportNumber = 5678;

    static void Main(string[] args)
    {
        ChatServer server = new ChatServer(_kportNumber);

        server.Status += (s, e) => WriteLine(e.StatusText);

        Task serverTask = _WaitForServer(server);

        WriteLine("Press return to shutdown server...");
        ReadLine();

        server.Shutdown();
        serverTask.Wait();
    }

    private static async Task _WaitForServer(ChatServer server)
    {
        try
        {
            await server.ListenTask;
        }
        catch (Exception e)
        {
            WriteLine($"Server exception: {e}");
        }
    }
}

А вот и клиент:

class Program
{
    private const int _kportNumber = 5678;

    static void Main(string[] args)
    {
        IPEndPoint remoteEndPoint = new IPEndPoint(IPAddress.Loopback, _kportNumber);
        ConnectedEndPoint server = ConnectedEndPoint.Connect(remoteEndPoint, (c, s) => WriteLine(s));

        _StartUserInput(server);
        _SafeWaitOnServerRead(server).Wait();
    }

    private static void _StartUserInput(ConnectedEndPoint server)
    {
        // Get user input in a new thread, so main thread can handle waiting
        // on connection.
        new Thread(() =>
        {
            try
            {
                string line;

                while ((line = ReadLine()) != "")
                {
                    server.WriteLine(line);
                }

                server.Shutdown();
            }
            catch (IOException e)
            {
                WriteLine($"Server {server.RemoteEndPoint} IOException: {e.Message}");
            }
            catch (Exception e)
            {
                WriteLine($"Unexpected server exception: {e}");
                Environment.Exit(1);
            }
        })
        {
            // Setting IsBackground means this thread won't keep the
            // process alive. So, if the connection is closed by the server,
            // the main thread can exit and the process as a whole will still
            // be able to exit.
            IsBackground = true
        }.Start();
    }

    private static async Task _SafeWaitOnServerRead(ConnectedEndPoint server)
    {
        try
        {
            await server.ReadTask;
        }
        catch (IOException e)
        {
            WriteLine($"Server {server.RemoteEndPoint} IOException: {e.Message}");
        }
        catch (Exception e)
        {
            // Should never happen. It's a bug in this code if it does.
            WriteLine($"Unexpected server exception: {e}");
        }
    }
}

На мой взгляд, одна из самых важных вещей, которую вы должны отметить в приведенном выше, состоит в том, что классы ConnectedEndPoint и ChatServer имеют нулевая зависимость от классов, которые их используют. Благодаря использованию делегатов обратного вызова и событий код, зависящий от этих классов, может взаимодействовать двунаправленно без того, чтобы эти поддерживающие классы знали о типах, в которых находится код (см. "инверсия управления", которая является вариацией).

Чем больше вы можете сделать так, чтобы ваши отношения кода выглядели как дерево с только однонаправленными ссылками, тем легче будет написать код, а также поддерживайте его позже.

Примечание: я использовал оба события и делегатов обратного вызова для иллюстрации. Любой подход прекрасно работает сам по себе. Основными компромиссами являются сложность и гибкость. Использование событий делает код более гибким - обработчики событий могут быть добавлены и удалены по мере необходимости - но если реализовать события с помощью .NET-соглашения сигнатуры метода с параметром sender и EventArgs, это несколько более "тяжеловесно", чем просто передача простого делегата обратного вызова при создании рассматриваемого объекта. Я привожу пример каждого из них в коде, и вы можете решить, какие подходы вы предпочитаете в каких ситуациях.

Вы также заметите, что вышеизложенное интенсивно использует асинхронные функции C#. Во-первых, это может сделать код, кажется, трудно читать. Но на самом деле, на самом деле гораздо проще заставить все работать, используя эти функции, чем если бы я попытался использовать старые BeginXXX()/EndXXX() методы или, не дай бог, посвятить каждому отдельную нить соединение (которое масштабируется очень плохо по мере увеличения количества клиентов). Безусловно, стоит привыкнуть думать об операциях, которые по своей природе асинхронны, таких как сетевой ввод/вывод, Таким образом.