Проблемы с многопоточностью в Java HashMap - PullRequest
3 голосов
/ 28 октября 2008

Что-то случилось, что я не уверен, что это возможно. Очевидно, потому что я видел это, но мне нужно найти основную причину, и я надеялся, что вы все можете помочь.

У нас есть система, которая ищет широту и долготу для почтового индекса. Вместо того, чтобы обращаться к нему каждый раз, мы кешируем результаты в дешевом кеше HashTable в памяти, поскольку широта и длина почтового индекса имеют тенденцию меняться реже, чем мы выпускаем.

В любом случае, хеш окружен классом, в котором есть методы get и add, которые синхронизированы. Мы получаем доступ к этому классу как к одиночке.

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

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

Совсем недавно у нас произошел инцидент, когда два разных почтовых индекса вернули одно и то же значение. Предполагая, что начальные значения на самом деле были разными, есть ли способ, что запись значений в карту приведет к тому, что одно и то же значение будет записано для двух разных ключей? Или есть ли способ, при котором 2 «получает» может пересечь провода и случайно вернуть то же значение?

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

Любые идеи будут оценены. Спасибо, Питер

(PS: дайте мне знать, если вам нужна дополнительная информация, код и т. Д.)

public class InMemoryGeocodingCache implements GeocodingCache
{

private Map cache = new HashMap();
private static GeocodingCache instance = new InMemoryGeocodingCache();

public static GeocodingCache getInstance()
{
    return instance;
}

public synchronized LatLongPair get(String zip)
{
    return (LatLongPair) cache.get(zip);
}

public synchronized boolean has(String zip)
{
    return cache.containsKey(zip);
}

public synchronized void add(String zip, double lat, double lon)
{
    cache.put(zip, new LatLongPair(lat, lon));
}
}


public class LatLongPair {
double lat;
double lon;

LatLongPair(double lat, double lon)
{
    this.lat = lat;
    this.lon = lon;
}

public double getLatitude()
{
    return this.lat;
}

public double getLongitude()
{
    return this.lon;
}
}

Ответы [ 8 ]

8 голосов
/ 28 октября 2008

Код выглядит правильно.

Единственная проблема заключается в том, что lat и lon видимы в пакете, поэтому для одного и того же кода пакета возможно следующее:

LatLongPair llp = InMemoryGeocodingCache.getInstance().get(ZIP1);
llp.lat = x;
llp.lon = y;

, что явно изменит объект в кэше.

Так что сделайте лат и лон финальными тоже.

P.S. Поскольку ваш ключ (почтовый индекс) является уникальным и небольшим, нет необходимости вычислять хэш для каждой операции. Проще использовать TreeMap (обернутый в Collections.synchronizedMap ()).

P.P.S. Практический подход: написать тест для двух потоков, выполняющих операции put / get в бесконечном цикле, проверяя результат при каждом получении. Для этого вам понадобится многопроцессорная машина.

6 голосов
/ 28 октября 2008

Почему это происходит, сказать сложно. Может помочь больше кода.

Вероятно, вы все равно должны просто использовать ConcurrentHashMap. В целом это будет более эффективно, чем синхронизированная карта. Вы не синхронизируете доступ к нему, он обрабатывает его внутренне (более эффективно, чем вы могли бы).

4 голосов
/ 28 октября 2008

Следует обратить внимание на то, что ключ или значение могут изменяться, например, если вместо создания нового объекта для каждой вставки вы просто изменяете значения существующего объекта и вставляете его заново.

Вы также хотите убедиться, что ключевой объект определяет hashCode и equals таким образом, чтобы не нарушать контракт HashMap (т. Е. Если equals возвращает true, hashCodes должны быть одинаковыми, но не обязательно наоборот Versa).

3 голосов
/ 28 октября 2008

возможно ли изменить LatLonPair? Я бы предложил сделать поля lat и lon окончательными, чтобы они не были случайно изменены в другом месте кода.

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

2 голосов
/ 28 октября 2008

Джеймс прав. Поскольку вы возвращаете объект, его внутренние компоненты могут быть изменены, и все, что содержит ссылку на этот объект (карту), будет отражать это изменение. Финал - хороший ответ.

0 голосов
/ 14 ноября 2013

Вот документ Java на HashMap:

http://docs.oracle.com/javase/7/docs/api/java/util/HashMap.html

Обратите внимание, что эта реализация не синхронизирована. Если несколько потоков обращаются к хэш-карте одновременно, и хотя бы один из потоков структурно изменяет карту, она должна быть синхронизирована извне. (Структурная модификация - это любая операция, которая добавляет или удаляет одно или несколько сопоставлений; простое изменение значения, связанного с ключом, который уже содержится в экземпляре, не является структурной модификацией.) Обычно это выполняется путем синхронизации с некоторым объектом, который естественным образом инкапсулирует карту , Если такого объекта не существует, карту следует «обернуть» с помощью метода Collections.synchronizedMap. Это лучше всего делать во время создания, чтобы предотвратить случайный несинхронизированный доступ к карте:

Карта m = Collections.synchronizedMap (новый HashMap (...));

Или лучше использовать java.util.concurrent.ConcurrentHashMap

0 голосов
/ 06 ноября 2008

Наличие имеет (String ZIP) метод подразумевает, что у вас есть что-то вроде следующего в вашем коде:

GeocodingCache cache = InMemoryGeocodingCache.getInstance();

if (!cache.has(ZIP)) {
    cache.add(ZIP, x, y);
}

К сожалению, это открывает вам возможность синхронизировать проблемы между has (), возвращающим false, и добавлением add (), которое может привести к описанной вами проблеме.

Лучшим решением было бы переместить проверку в метод add, чтобы проверка и обновление были покрыты одним и тем же замком, например:

public synchronized void add(String zip, double lat, double lon) {
    if (cache.containsKey(zip)) return;
    cache.put(zip, new LatLongPair(lat, lon));
}

Еще одна вещь, которую я должен упомянуть, это то, что если вы используете getInstance () как синглтон, у вас должен быть приватный конструктор, чтобы предотвратить возможность создания дополнительных кэшей с использованием new InMemoryGeocodingCache () .

0 голосов
/ 29 октября 2008

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

Другие вещи, которые нужно учитывать (некоторые из них довольно очевидны, но я решил, что все равно укажу на них):

  1. С какими двумя почтовыми индексами у вас были проблемы? Вы уверены, что у них нет идентичных геокодов в исходной системе?
  2. Вы уверены, что случайно не сравниваете два одинаковых почтовых индекса?
Добро пожаловать на сайт PullRequest, где вы можете задавать вопросы и получать ответы от других членов сообщества.
...