Имеется ситуация, в которой оказалось удобно использовать код вида

synchronized (key.toString().intern()) {
   ...
}

но многие пишут, что так не стоит делать, это засорение intern-пула, это теоретический дедлок, если в другом месте случайно по той же строке будет синхронизация и тд. Вопрос — чем заменить этот код? Пока написал как-то так (Java 7):

ConcurrentMap<Key, Object> locks = new ConcurrentHashMap<>();
...
        Object lock = locks.get(key);
        if (lock == null) {
            Object newLock = new Object();
            Object existingLock = locks.putIfAbsent(key, newLock);
            lock = existingLock == null ? newLock : existingLock;
        }
        synchronized (lock) {
            ...
        }

но как-то это всё сложно выглядит. Не упускаю ли я какой-то готовый примитив синхронизации? Помимо прочего тут ещё и locks будет расти, в моём случае я её чищу сразу после synchronized, потому что у меня этот блок выполняется один раз, но в общем случае так нельзя делать, я сходу даже не соображу, как правильно это написать для общего случая.

Здравствуйте, vsb, Вы писали:

vsb>ConcurrentMap<Key, Object> locks = new ConcurrentHashMap<>();
vsb>...
vsb>        Object lock = locks.get(key);
vsb>        if (lock == null) {
vsb>            Object newLock = new Object();
vsb>            Object existingLock = locks.putIfAbsent(key, newLock);
vsb>            lock = existingLock == null ? newLock : existingLock;
vsb>        }
vsb>        synchronized (lock) {
vsb>            ...
vsb>        }

vsb>но как-то это всё сложно выглядит.

Не знаю, можно ли другим примитивом, но если делать так, как написано, то тебе надо использовать computeIfAbsent, который для ConcurrentHashMap — атомарный, т.о. избавишься от громоздкости.

Здравствуйте, C0s, Вы писали:

C0s>Не знаю, можно ли другим примитивом, но если делать так, как написано, то тебе надо использовать computeIfAbsent, который для ConcurrentHashMap — атомарный, т.о. избавишься от громоздкости.

В Java 7 его нет, к сожалению.

vsb>но многие пишут, что так не стоит делать, это засорение intern-пула,

Начиная с Java7 этот пул в обычном хипе и, соответственно, там даже мусор собирается.

vsb>теоретический дедлок, если в другом месте случайно по той же строке будет синхронизация и тд.

Вот это верно — никогда нельзя делать синхронизацию на объектах, жизненный цикл которых ты не контролируешь от и до.

vsb>но как-то это всё сложно выглядит. Не упускаю ли я какой-то готовый примитив синхронизации?

Не упускаешь

vsb>Вопрос — чем заменить этот код? Пока написал как-то так (Java 7):

А чего этот код делает? Именованный лок, который много раз может лочиться и отпускаться? Нормально тогда.

В общем, если кому надо, накидал такой код, вроде должен работать. Поискал альтернативы, по сути примерно так все делают, для оптимизации производительности можно держать не один глобальный лок на map, а список (выбирать по хешу ключа), но тогда надо вместо HashMap ConcurrentHashMap использовать.

import java.util.HashMap;
import java.util.Map;
import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.ReentrantLock;

class LockMap {
    interface Unlockable {
        void unlock();
    }

    private final Lock locksLock = new ReentrantLock();
    private final Map<Object, ReferenceCountingLock> locks = new HashMap<Object, ReferenceCountingLock>();

    Unlockable lock(Object key) {
        boolean lockSuccessful = false;
        ReferenceCountingLock rcl = acquire(key);
        try {
            Unlocker unlocker = new Unlocker(key, rcl);
            rcl.lock.lock();
            lockSuccessful = true;
            return unlocker;
        } finally {
            if (!lockSuccessful) {
                release(key, rcl);
            }
        }
    }

    private ReferenceCountingLock acquire(Object key) {
        locksLock.lock();
        try {
            ReferenceCountingLock rcl = locks.get(key);
            if (rcl == null) {
                rcl = new ReferenceCountingLock();
                locks.put(key, rcl);
            }
            rcl.count++;
            return rcl;
        } finally {
            locksLock.unlock();
        }
    }

    private void release(Object key, ReferenceCountingLock rcl) {
        locksLock.lock();
        try {
            rcl.count--;
            if (rcl.count == 0) {
                ReferenceCountingLock oldRCL = locks.remove(key);
                if (rcl != oldRCL) {
                    throw new IllegalStateException();
                }
            }
        } finally {
            locksLock.unlock();
        }
    }

    private static class ReferenceCountingLock {
        private int count;
        private final Lock lock = new ReentrantLock();
    }

    private class Unlocker implements Unlockable {
        private final Object key;
        private final ReferenceCountingLock rcl;

        private Unlocker(Object key, ReferenceCountingLock rcl) {
            this.key = key;
            this.rcl = rcl;
        }

        @Override
        public void unlock() {
            try {
                rcl.lock.unlock();
            } finally {
                release(key, rcl);
            }
        }
    }
}

Использовать так же, как обычные локи:

        Unlockable lock = lockMap.lock(key);
        try {
           ...
        } finally {
            lock.unlock();
        }

Здравствуйте, vsb, Вы писали:

Если допустимы коллизии, то можно посмотреть в сторону Striped от guava:

String key = "taskA";
ReadWriteLock rwLock = rwLockStripes.get(key);
try{
     rwLock.lock();
     .....
}finally{
     rwLock.unLock();
}

Чего-то ты переусложнил, особенно ручное управление жизнью объекта зачем-то сделал

import java.util.concurrent.locks.ReentrantLock;

import com.google.common.annotations.VisibleForTesting;
import com.google.common.cache.CacheBuilder;
import com.google.common.cache.CacheLoader;
import com.google.common.cache.LoadingCache;

public class FineGrainedLockCache {

    private final LoadingCache<Object, AutoReleasedLock> locks = CacheBuilder.newBuilder()
        .weakValues()
        .build(CacheLoader.from(k -> new AutoReleasedLock(true)));

    public AutoReleasedLock getLock(Object id) {
        return locks.getUnchecked(id);
    }

    public AutoReleasedLock lock(Object id) {
        final AutoReleasedLock lock = getLock(id);
        lock.lock();
        return lock;
    }

    @VisibleForTesting
    boolean isAvailable(Object id) {
        return locks.getIfPresent(id) != null;
    }

    public static class AutoReleasedLock extends ReentrantLock implements AutoCloseable {

        AutoReleasedLock(boolean fair) {
            super(fair);
        }
        @Override
        public void close() {
            unlock();
        }
    }
}

FineGrainedLockCache  locks = new FineGrainedLockCache ();
....

try (locks.lock("some-key")) {
  ....
}

Решение такой проблемы есть в книжке Java Concurrency In Practice, называется Memorizer. С виду оне конечно замороченное, на работать будет и на java 1.5

public class Memorizer<A, V> implements Computable<A, V> {
    private final ConcurrentMap<A, Future<V>> cache
            = new ConcurrentHashMap<A, Future<V>>();
    private final Computable<A, V> c;
    public Memorizer(Computable<A, V> c) { this.c = c; }
    public V compute(final A arg) throws InterruptedException {
        while (true) {
            Future<V> f = cache.get(arg);
            if (f == null) {
                Callable<V> eval = new Callable<V>() {
                    public V call() throws InterruptedException {
                        return c.compute(arg);
                    }
                };
                FutureTask<V> ft = new FutureTask<V>(eval);
                f = cache.putIfAbsent(arg, ft);
                if (f == null) { f = ft; ft.run(); }
            }
            try {
                return f.get();
            } catch (CancellationException e) {
                cache.remove(arg, f);
            } catch (ExecutionException e) {
                throw launderThrowable(e.getCause());
            }
        }
    }
}