Безопасность потоков при итерации через одновременные коллекции

StackOverflow https://stackoverflow.com/questions/3950354

Вопрос

Я пишу немного клиент-сервер-приложение, где мне нужно иметь дело с несколькими потоками. У меня есть несколько серверов, которые отправляют живые пакеты каждые несколько секунд. Эти серверы поддерживаются в CONCULLENTASHMAP, который содержит свои конечные точки в паре со временем, когда последний Alive-Package приехал соответствующего сервера.

Теперь у меня есть нить, которая должна «разбирать» все серверы, которые не отправили в живые пакеты на определенное время.

Я думаю, я не могу просто сделать это, могу ли я?

for( IPEndPoint server : this.fileservers.keySet() )
{
    Long time = this.fileservers.get( server );

    //If server's time is updated here, I got a problem

    if( time > fileserverTimeout )
        this.fileservers.remove( server );
}

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

Это было полезно?

Решение

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

Моя первая идея для записи о том, что данные должны были хранить «Последние временные метки, на которых был активен сервер». Тогда ваш код будет выглядеть так:

package so3950354;

import java.util.Iterator;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;

public class ServerManager {

  private final ConcurrentMap<Server, Long> lastActive = new ConcurrentHashMap<Server, Long>();

  /** May be overridden by a special method for testing. */
  protected long now() {
    return System.currentTimeMillis();
  }

  public void markActive(Server server) {
    lastActive.put(server, Long.valueOf(now()));
  }

  public void removeInactive(long timeoutMillis) {
    final long now = now();

    Iterator<Map.Entry<Server, Long>> it = lastActive.entrySet().iterator();
    while (it.hasNext()) {
      final Map.Entry<Server, Long> entry = it.next();
      final long backThen = entry.getValue().longValue();
      /*
       * Even if some other code updates the timestamp of this server now,
       * the server had timed out at some point in time, so it may be
       * removed. It's bad luck, but impossible to avoid.
       */
      if (now - backThen >= timeoutMillis) {
        it.remove();
      }
    }
  }

  static class Server {

  }
}

Если вы действительно хотите избежать того, чтобы код не звонил markActive во время звонка removeInactive, нет никакого пути вокруг явного блокировки. Что вы, вероятно, хотите:

  • одновременные звонки в markActive разрешены.
  • в течение markActive нет звонков в removeInactive разрешены.
  • в течение removeInactive нет звонков в markActive разрешены.

Это похоже на типичный сценарий для ReadWriteLock, куда markActive это операция «чтения» и removeInactive это «написание» операция.

Другие советы

Я не вижу, как еще одна тема может обновить время сервера в тот момент в вашем коде. После того, как вы получили время сервера с карты, используя this.fileservers.get( server ), Другой нить не может изменить его значение, так как длинные объекты неизменяются. Да, другая нить может поставить новый длинный объект для этого сервера на карту, но это не влияет на этот поток, потому что она уже извлекла время сервера.

Так как он стоит, я не вижу ничего плохого в вашем коде. Итераторы в ConcurrenthAshmap слабо согласуется Что означает, что они могут терпеть одновременную модификацию, так что нет риска, рискующего о компенсификации.

(Видеть Ответ Роланда, который принимает идеи здесь и мясирует их в более полный пример, с некоторыми великими дополнительными идеями.)

Поскольку это параллельная хеш-карта, вы можете сделать следующее. Обратите внимание, что итераторы CMM все реализуют дополнительные методы, в том числе remove(), какой ты хочешь. Видеть CHM API Документы, в котором говорится:

Этот класс и его взгляды и итераторы реализуют все дополнительные методы Map а также Iterator интерфейсы.

Этот код должен работать (я не знаю тип Key В вашем CHM):

ConcurrentHashMap<K,Long> fileservers = ...;

for(Iterator<Map.Entry<K,Long>> fsIter = fileservers.entrySet().iterator(); fileservers.hasNext(); )
{
    Map.Entry<K,Long> thisEntry = fsIter.next();
    Long time = thisEntry.getValue();

    if( time > fileserverTimeout )
        fsIter.remove( server );
}

Но обратите внимание, что в других местах могут быть в другом месте ... вам необходимо убедиться, что другие биты кода доступ к карте, могут справиться с таким видом спонтанного удаления - то есть, вероятно, при этом fileservers.put() Вам понадобится немного логики с участием fileservers.putIfAbsent(). Отказ Это решение с меньшей вероятностью создает узкие места, чем использование synchronized, но это также требует немного больше мыслей.

Где вы написали «если время сервера обновляется здесь, у меня проблема» - это именно где putIfAbsent() входит. Если запись отсутствует, либо вы не видели этого раньше, либо вы только что недавно отбросили его из таблицы. Если две стороны этого необходимо скоординировать, то вы можете вместо этого хотеть ввести запираемую запись для записи и выполнять синхронизацию на этом уровне (т. Е. Синхронизация на записи при выполнении remove(), а не на весь стол). Тогда put() Конец вещей также может синхронизировать на одной и той же записи, устраняя потенциальную гонку.

Во-первых, сделать карту синхронизированной

this.fileservers = Collections.synchronizedMap(Map)

Затем используйте стратегию, которая используется в классах Singleton 

if( time > fileserverTimeout )
    {
        synchronized(this.fileservers)
        {
             if( time > fileserverTimeout )
                  this.fileservers.remove( server );
        }
    }

Теперь это гарантирует, что когда-то вы внутри синхронизированного блока не могут возникнуть обновления. Это так, потому что после того, как замок на карте сделан, карта (синхронизированная обертка) не будет доступен для обеспечения блокировки потоков для обновления, удаления и т. Д.

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

Лицензировано под: CC-BY-SA с атрибуция
Не связан с StackOverflow
scroll top