Синхронизация по Integer приводит к NullPointerException

Этот вопрос основан на синхронизации по целочисленному значению.

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

Так что заняться приведенной ниже программой

import java.util.concurrent.ConcurrentHashMap;

public class Example {

    private final ConcurrentHashMap<Integer, Integer> concurrentHashMap = new ConcurrentHashMap<Integer, Integer>();

    public void doSomething(int i) {
        synchronized (getLockForId(i)) {
            concurrentHashMap.remove(i);
        }
    }

    public Integer getLockForId(int id) {
        concurrentHashMap.putIfAbsent(id, id); // I want to replace these two
                                                // operation with single one
                                                // since it seems the cause of
                                                // NPE
        return concurrentHashMap.get(id);
    }

    public static void main(String[] args) {
        final Example example = new Example();
        new Thread(new Runnable() {
            @Override
            public void run() {
                int i = 0;
                while (true) {
                    example.doSomething(++i);
                }
            }
        }).start();
        new Thread(new Runnable() {
            @Override
            public void run() {
                int i = 0;
                while (true) {
                    example.doSomething(++i);
                }
            }
        }).start();
    }
}

Проблема в том, что это всегда приводит к NullPointerException, Мой первый анализ был, потому что я удаляю значение, которое ему присваивается, так что это вызывает NullPointerException, Так я и сделал ниже

    Object obj = new Object();
    synchronized (obj) {
        obj = null;
    }

Но выше не приводит к NullPointerException, Так что мой вопрос, почему это бросать NullPointerException в вышеуказанном случае?

Даже если я сделаю

public Integer getLockForId(int id) {
   return concurrentHashMap.putIfAbsent(id, id); 
}

Это все еще приводит к NullPointerException потому что он возвращает значение только тогда, когда есть еще один возврат null

3 ответа

Решение

Ну да, это бросило бы NullPointerException, Рассмотрим эту схему:

 Thread 1                     Thread 2

 putIfAbsent (puts)
 get (returns non-null)
 acquire monitor
                              putIfAbsent (doesn't put)
 remove (removes value)
                              get (returns null)
                              acquire monitor (bang!)

Дело не в том, что "значение присваивается нулю" - это Map.get возвращает ноль, если для данного ключа нет записи.

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

РЕДАКТИРОВАТЬ: Как отметил Никита, просто возвращая значение putIfAbsent не работает, так как это возвращает предыдущее значение, или null если он отсутствовал - тогда вы хотите новое значение для записи.

Я подозреваю, что вам придется синхронизировать доступ к карте, чтобы getLockId Метод атомарный по отношению к remove операция.

Вы можете попытаться сделать все доступы к concurrentHashMap синхронный. Поэтому, когда вы получаете значение из карты, вы синхронизируете на concurrentHashMap и когда вы удалите его. Что-то вроде этого:

public void doSomething(int i) {
    synchronized (getLockForId(i)) {
        // do stuff
        synchronized (concurrentHashMap) {
            concurrentHashMap.remove(i);
        }
    }
}


public Integer getLockForId(int id) {
    synchronized (concurrentHashMap) {
        concurrentHashMap.putIfAbsent(id, id);
        return concurrentHashMap.get(id);
    }
}

Попробуйте использовать Google LoadingCache вместо этого. Он использует слабые ссылки, поэтому сборка мусора оставлена ​​на усмотрение JVM; Вам не нужно удалять ссылки, которые вам больше не нужны. И это решает проблемы параллелизма, связанные с созданием новой записи. Код для синхронизатора значений будет выглядеть так:

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

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

public class ValueSynchronizer<T> {
  private final LoadingCache<T, Lock> valueLocks;

  public ValueSynchronizer() {
    valueLocks = CacheBuilder.newBuilder().build(
      new CacheLoader<T, Lock>() {
        public Lock load(final T id) {
          return new ReentrantLock();
        }
      });
  }

  public void sync(final T onValue, final Runnable toDo)
  {
    final Lock lock = valueLocks.getUnchecked(onValue);
    lock.lock();

    try {
      toDo.run();
    }
    finally {
      lock.unlock();
    }
  }
}

На самом деле синхронизация по значению будет выглядеть так:

private final ValueSynchronizer<Long> retrySynchronizer 
    = new ValueSynchronizer<>();

@Override
public void onApplicationEvent(final EventThatCanBeSentMultipleTimes event)
{
  retrySynchronizer.sync(event.getEventId(), () ->
  {
    //...Your code here.
  });
}

(лицензируется по лицензии Apache 2.0: http://www.apache.org/licenses/LICENSE-2.0)

Другие вопросы по тегам