2015-12-18 5 views
4

В интервью мне было предложено проверить, работает ли следующий код в порядке?ConcurrentHashmap - читать и удалять

ConcurrentHashMap<Integer, Integer> chm = new ConcurrentHashMap<Integer, Integer>(); 

    if(chm.get(key) != null) { 
     chm.get(key).doSomething(); 
     chm.remove(key); 
    } 

Согласно Javadocs, «get» возвращает значение последней завершенной операции обновления. Поэтому, если поток 1 уже называется chm.remove (key), и если поток 2 входит в оператор if и собирается вызвать метод get, мы можем получить исключение. Это верно?

Последующий вопрос: как я могу сделать эту тему безопасной?

ответ

2

Вы верны. Если этот Map может быть изменен несколькими потоками, возможно, что первый вызов chm.get(key) вернет ненулевое значение, а второй вызов вернет null (из-за удаления ключа из Map, выполненного другим потоком) и таким образом, chm.get(key).doSomething() будет бросать NullPointerException.

Вы можете сделать этот код потокобезопасными, используя локальную переменную для хранения результата chm.get(key):

ConcurrentHashMap<Integer, Integer> chm = new ConcurrentHashMap<Integer, Integer>(); 
Integer value = chm.get(key); 

if(value != null) { 
    value.doSomething(); // P.S. Integer class doesn't have a doSomething() method 
         // but I guess this is just an example of calling some arbitrary 
         // instance method 
    chm.remove(key); 
} 

BTW, даже если это Map был не ConcurentHashMap и только один поток имел доступ к нему, Я бы по-прежнему использовал локальную переменную, поскольку она более эффективна, чем вызов метода get() дважды.

РЕДАКТИРОВАТЬ:

Как прокомментировал ниже, это исправление не помешает doSomething() от вызова несколько раз для одной и того же ключа/значения разных потоков. Независимо от того, является ли это желаемым поведением, неясно.

Если вы хотите, чтобы возможность doSomething() вызывалась несколькими потоками для одного и того же ключа/значения, вы можете использовать chm.remove(key) для удаления ключа и получения значения на том же шаге.

Это, однако, рискует тем, что doSomething() не будет выполняться вообще для какой-то ключ/значение, так как если первый вызов doSomething() привело к исключению, не будет другой вызов doSomething() другим потоком, так как пара ключей/значений больше не будет в Map. С другой стороны, если вы удалите пару ключ/значение с Карты только после успешного выполнения doSomething(), вы гарантируете, что doSomething() будет успешно выполнен хотя бы один раз для всех пар ключ/значение, которые были повторно выпущены из Map.

+0

Возможно, даже лучше просто написать 'Integer value = chm.remove (key);' и удалить существующую строку 'chm.remove (key);' полностью. Таким образом, 'value' не лежит на карте, пока вы вызываете' doSomething() '. –

+0

@ ErickG.Hagstrom Я не знаю, является ли проблема, что значение «лежит на карте, когда вы вызываете doSomething()». Возможно, требование состоит в том, чтобы удалить запись ключа/значения из Карты только после успешного выполнения doSomething. – Eran

+0

Это не Threadsafe, 'doSomething' можно вызвать более одного раза. –

3

Map.remove(key) возвращает значение, если оно было удалено. Это очень хороший трюк во многих ситуациях, в том числе ваших:

Object value = chm.remove(key) 
if(value != null) 
{ 
    value.doSomething(); 
} 

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

Это невозможно, если вы удалите его первым. Код выше - Threadsafe, также проще.

+2

Каким-то образом я чувствую, что интервьюер будет ожидать это как правильный ответ –