Говорить о клиент-серверных приложениях можно долго. Остановлюсь только на одном аспекте: как сделать так, чтобы этот сервер не лег.
Решения, которые у вас применены, вполне будут работать при нагрузке в 1 человека (поскольку одновременно в двух окнах мы кнопки нажимать не можем), но уже 2-3 человека смогут легко положить сервак, если начнут часто отправлять сообщения.
В связи с этим несколько пожеланий:
1. Отсутствует перехват исключения при подключении нового клиента:
В цикле ожидания подключений мы выполняем вызов AcceptTcpClient():
ServerClient client = new ServerClient(server.AcceptTcpClient());
При этом легко можем получить сетевое исключение, связанное с невозможностью подключения отдельного клиента. И наш сервер ляжет. Из-за одного единственного неудачного подключения клиента (может у него инет отвалился). А сервер должен продолжать работать. Для этого вызов AcceptTcpClient() имеет смысл завернуть в try-catch, логировать исключение и продолжить работу.
try
{
ServerClient client = new ServerClient(server.AcceptTcpClient());
// ...
}
catch (Exception e) when (server.Active)
{
// Логируем исключение и продолжаем работу
// (если сервер не остановлен).
}
2. Нужно добавить блокировку при использовании общих данных из разных потоков
Статическое поле Server.clients используется из разных потоков.
Получается, если два запроса придут одновременно, мы можем получить ситуацию, когда в разных потоках могут одновременно вызываться:
clients.Add()
clients.ForEach()
clients.TrueForAll()
clients.Remove()
На практике это приведет, в лучшем случае, к лишним исключениям, которые нам будет выбрасывать стандартная библиотека. А в худшем случае, если вызвать одновременно clients.Add и clients.Remove из разных потоков, — данные этого списка будут разрушены.
Чтобы подобного не произошло, имеет смысл при ЛЮБОМ доступе к полю clients использовать блокировку.
Например, так:
public static class Server
{
public static object State = new object();
public static void Listen(string ip)
{
//...
// Кстати, цикл имеет смысл прервать, если сервер будет остановлен через Stop()
while (server.Active)
{
//...
lock(State)
{
clients.Add(client);
}
}
}
}
public class ServerClient
{
private void RemoveUser()
{
lock(Server.State)
{
Server.clients.Remove(this);
Server.users = Server.users.Replace(name + "\n", "");
}
}
}
В этом случае, если один поток начал что-то делать внутри "lock(State)", другой на входе в эту блокировку будет ждать, пока первый не закончит свою работу.
Если нам нужно работать сразу с несколькими разделяемыми ресурсами, например, "clients" и "users", эту работу имеет смысл поместить в единый блок lock(State) — см. пример выше.
Как минимум, поле "clients" еще используется в Server.BroadcastMessage(), Server.isUniqueName(), ServerClient.AddUser(). Там тоже следует использовать блокировку.
3. Отсутствует перехват возможного исключения при отправке сообщения клиенту
В методе ServerClient.SendMessage() при вызове:
await stream.WriteAsync()
— запросто может произойти исключение из-за дефекта связи. В этом случае наш сервер ляжет. Чтобы этого не произошло, поступаем также, как в StartReceiving() — заворачиваем в try-catch.
А вообще — для учебного проекта вполне прилично. Вы продемонстрировали способность решения сложных задач простыми способами с минимальным количеством кода. Это очень пригодится в карьере.
Сложный
