Будет ли этот метод работать потокобезопасным и без тупиков

Question

public int saveUserToMap(User user) {
    ReentrantLock lock;
    if(this.userLocks.containsKey(user.getId())) {
        lock = this.userLocks.get(user.getId());
    } else {
        lock = new ReentrantLock();
        ReentrantLock check = this.userLocks.putIfAbsent(user.getId(), lock);
        if(check != null)
            lock = check;
    }

    if(lock.isLocked())
        try {
            lock.wait();
        } catch (InterruptedException e) {
            // TODO Auto-generated catch block
            e.printStackTrace();
        }

    lock.lock();

    this.users.put(user.getId(), user);
    this.usersByName.put(user.getUsername(), user);
    this.usersByEmail.put(user.getEmail(), user);

    lock.unlock();
    lock.notify();

    return user.getId();
}

Привет, я просто хочу попросить разработчиков java проверить мой код, будет ли он поточно-ориентированным и свободным от тупиковых ситуаций, поскольку я хочу использовать его в своем проекте.Users, UsersByName и UsersByEmail - это ConcurrentHashMap с String, Integer в качестве ключа и объект User в качестве значения.UserLocks - это ConcurrentHashMap с Integer (очевидно, идентификатор пользователя в качестве ключа) и ReentrantLock в качестве значения.Я хочу синхронизировать три HashMaps.Если у кого-то есть лучшее решение для создания карты Concurrent с тремя ключами, было бы неплохо опубликовать ее здесь.Производительность также важна.

Answer 1

Я бы сделал это простым способом, используя synchronized.

class UserMaps {
    private Map<Integer, User> users = new ConcurrentHashMap<>();
    private Map<String, User> usersByName = new ConcurrentHashMap<>();
    private Map<String, User> usersByEmail = new ConcurrentHashMap<>();

    public synchronized int put(User user) {
        users.put(user.getId(), user);
        usersByName.put(user.getUsername(), user);
        usersByEmail.put(user.getEmail(), user);
        return user.getId();
    }
}

. Это обеспечило бы постоянное обновление всех карт, если все ваши получатели также synchronized.

*.1007 * Если, однако, вы хотите повысить производительность и не хотите всех ваших методов synchronized, используйте ReadWriteLock.

class UserMaps {
    ReentrantReadWriteLock lock = new ReentrantReadWriteLock();
    private Map<Integer, User> users = new ConcurrentHashMap<>();
    private Map<String, User> usersByName = new ConcurrentHashMap<>();
    private Map<String, User> usersByEmail = new ConcurrentHashMap<>();

    public int saveUserToMap(User user) {
        lock.writeLock().lock();
        try {
            users.put(user.getId(), user);
            usersByName.put(user.getUsername(), user);
            usersByEmail.put(user.getEmail(), user);
            return user.getId();
        } finally {
            lock.writeLock().unlock();
        }
    }

    public User getById(int id) {
        lock.readLock().lock();
        try {
            return users.get(id);
        } finally {
            lock.readLock().unlock();
        }
    }
}

Answer 2

Это потокобезопасно.

Если userId уже находится на карте, код получает блокировку и использует ее для синхронизации.Если нет, то ConcurrentHashMap обеспечивает синхронизацию, чтобы избежать состояния гонки для использования разных блокировок для одного и того же идентификатора.

После этого есть бесполезный фрагмент кода, от которого можно избавиться:

if(lock.isLocked())
    try {
        lock.wait();
    } catch (InterruptedException e) {
        // TODO Auto-generated catch block
        e.printStackTrace();
    }

Это не нужно, потому что синхронизация выполняется с помощью lock.lock().Нет необходимости пытаться снова выполнить синхронизацию, используя wait() и notify() с объектом блокировки. (На самом деле, это не работает так, как вы ожидали, несколько потоков могут вызывать lock.isLocked() для одного и того же объекта блокировки и получать false до тех пор, покаиз потоков вызывает lock.lock(), но все между блокировкой и разблокировкой выполняется только одним потоком за раз).

Кроме того, обычно рекомендуется вызывать lock.unlock() в блоке finally.