Многопоточный Java-сервер: предоставление одному потоку доступа к другому

Question

Надеюсь, сам код объясняет проблему здесь:

class Server {

    public void main() {
        // ...
        ServerSocket serverSocket = new ServerSocket(PORT);
        while (true) {
            Socket socket = serverSocket.accept();
            Thread thread = new Thread(new Session(socket));
            thread.start();
        }
        // ..
    }

    public static synchronized Session findByUser(String user) {
        for (int i = 0; i < sessions.size(); i++) {
            Session session = sessions.get(i);
            if (session.getUserID().equals(user)) {
                return session;
            }
        }
        return null;
    }

}

class Session {
    public Session(Socket socket) {
        attach(socket);
    }

    public void attach(Socket socket) {
        // get socket's input and output streams
        // start another thread to handle messaging (if not already started)
    }

    public void run() {
        // ...
        // user logs in and if he's got another session opened, attach to it
        Session session = Server.findByUser(userId);
        if (session != null) {
            // close input and output streams
            // ...
            session.attach(socket);
            return;
        }

        // ..

    }
}

Мой вопрос здесь такой: Безопасно ли публиковать ссылку на сеанс в методе Server.findByUser, не нарушает ли он стиль ООП и т. Д.? Или я должен ссылаться на сессии через некоторый неизменный идентификатор и инкапсулировать все это? Что-нибудь еще, что вы могли бы изменить здесь?

String sessionId = Server.findByUser(userId);
if (sessionId != null && sessionId.length() > 0) {
    // close input and output streams
    // ...
    Server.attach(sessionId, socket);
    return;
}

Томас:

Спасибо за ваш ответ.

Я согласен, в реальном мире было бы неплохо использовать внедрение зависимостей при создании нового экземпляра Session, но, возможно, также с интерфейсом, верно (код ниже) ? Несмотря на то, что у меня, вероятно, должны быть модульные тесты для этого, давайте рассмотрим, нет ли у меня. Тогда мне нужен ровно один экземпляр Сервера. Тогда было бы огромным ОО преступлением использовать статические методы вместо синглета?

interface Server {
    Session findByUser(String user);
}

class ServerImpl implements Server {
    public Session findByUser(String user) { }
}

class Session {
   public Session(Server server, Socket socket) { }
} 

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

class Session {
    public Session(Socket socket) {
       attach_socket(socket);
    }

    public void attach(Socket socket) {
        attach_socket(socket);
    }

    private void attach_socket(Socket socket) {
        // ...
    }
}

Это правда, что разрешать клиентам Session звонить attach(...) кажется неправильным. Это, вероятно, один из тех серьезных методов, к которым должен иметь доступ только Сервер. Как мне сделать это без отношений дружбы в C ++? Каким-то образом мне в голову пришли внутренние классы, но я об этом не задумывался, так что, возможно, это совершенно неверный путь.

Каждый раз, когда я получаю новое соединение, я создаю новый поток (и создаю новый экземпляр Session, связанный с ним) для обработки передачи. Таким образом, пока пользователь отправляет команду входа в систему, сервер готов принимать новые подключения. Как только личность пользователя подтверждена, я проверяю, не случайно ли он уже вошел в систему (еще один сеанс продолжается). Если он тогда, я отсоединяю сеанс onging от его сокета, закрываю этот сокет, присоединяю текущий сеанс к текущему сокету и закрываю текущий сеанс . Надеюсь, это более четкое объяснение того, что на самом деле происходит? Может быть, использование слова session немного неудачно. Что у меня действительно есть, так это 4 разных объекта, созданных для каждого соединения (и 3 потока): обработчик сокета, отправитель сообщения, получатель сообщения и сеанс (если это хорошее решение, это другой вопрос ...). Я просто попытался просто найти исходный код, чтобы сосредоточиться на вопросе.

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

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

Answer 1

Вы должны начать с создания класса OO сервера (т.е. не статического) и использовать внедрение зависимостей в классе Session:

class Server {
    public Session findByUser(String user) { }
}

class Session{
   public Session(Server server, Socket socket){}
}

public void attach(..) должен быть закрытым для обеспечения инкапсуляции и правильной инициализации. Подкласс может нарушить класс Session, иначе как это:

class BadSession extends Session{

@Override public void attach(Socket socket) {
    //this is not initialized at this point

    //now the instance is broken        
  }
}

Вызов вложения с клиента также кажется недействительным.

Ответственность за присоединение сокета к сеансу должна быть частью Сервера. Это правильное место, чтобы решить, какая сессия получит какой сокет. Насколько я понимаю ваш код, вы создаете сессию с сокетом. Каким-то образом вы узнаете, что у пользователя уже есть сеанс (с другим сокетом). Теперь вы присоединяете текущий сеанс к этому сокету. Теперь существует старый сокет с двумя сеансами и новый сокет без сеанса . Я думаю, что традиционная сессия должна иметь несколько сокетов, а не наоборот:

Session session = findSession(userId);
session.attach(socket);

class Session{
   List<Socket> sockets;
}

После этого изменения потоки будут назначаться не сеансам, а обработчикам сокетов, которые обрабатывают входной поток для одного сокета и соответственно изменяют сеанс.

Использование синхронизации для метода public static synchronized Session findByUser(String user) недостаточно для обеспечения безопасности потоков. Вы должны убедиться, что поиск сеанса (по пользователю) и регистрация сеанса (если пользователь не известен) должны быть atomic . Семантика должна быть аналогична putIfAbsent из ConcurrentMap. (В любом случае перебор списка сеансов неэффективен. Вы должны использовать Map<Id, Session>.)

Надеюсь, это поможет.