Поврежденные значения Java ConcurrentHashMap

У меня есть ConcurrentHashMap, который иногда ведет себя странно.

Когда мое приложение запускается впервые, я читаю каталог из файловой системы и загружаю содержимое каждого файла в ConcurrentHashMap, используя имя файла в качестве ключа. Некоторые файлы могут быть пустыми, и в этом случае я устанавливаю значение «пусто».

Как только все файлы будут загружены, пул рабочих потоков будет ожидать внешних запросов. Когда приходит запрос, я вызываю функцию getData(), где проверяю, содержит ли ConcurrentHashMap ключ. Если ключ существует, я получаю значение и проверяю, является ли значение «пустым». Если value.contains("пусто"), я возвращаю "файл не найден". В противном случае возвращается содержимое файла. Когда ключ не существует, я пытаюсь загрузить файл из файловой системы.

private String getData(String name) {
    String reply = null;
    if (map.containsKey(name)) {
        reply = map.get(name);
    } else {
        reply = getDataFromFileSystem(name);
    }

    if (reply != null && !reply.contains("empty")) {
        return reply;
    }

    return "file not found";
}

Иногда ConcurrentHashMap будет возвращать содержимое непустого файла (т. е. value.contains("empty") == false), однако строка:

if (reply != null && !reply.contains("empty")) 

возвращает ЛОЖЬ. Я разбил оператор IF на две части: if (reply != null) и if (!reply.contains("empty")). Первая часть оператора ЕСЛИ возвращает ИСТИНА. Вторая часть возвращает ЛОЖЬ. Поэтому я решил распечатать переменную «ответ», чтобы определить, действительно ли содержимое строки содержит «пусто». Это было НЕ так, т.е. содержимое не содержало строки «пусто». Кроме того, я добавил строку

int indexOf = reply.indexOf("empty");

Поскольку ответ переменной не содержал строки «пусто», когда я ее распечатывал, я ожидал, что indexOf вернет -1. Но функция вернула значение, приблизительно равное длине строки, то есть if reply.length == 15100, тогда reply.indexOf("empty") возвращало 15099.

Я сталкиваюсь с этой проблемой еженедельно, примерно 2-3 раза в неделю. Этот процесс перезапускается ежедневно, поэтому ConcurrentHashMap регулярно создается заново.

Кто-нибудь видел такое поведение при использовании Java ConcurrentHashMap?

ИЗМЕНИТЬ

private String getDataFromFileSystem(String name) {
    String contents = "empty";
    try {
        File folder = new File(dir);

        File[] fileList = folder.listFiles();
        for (int i = 0; i < fileList.length; i++) {
            if (fileList[i].isFile() && fileList[i].getName().contains(name)) {
                String fileName = fileList[i].getAbsolutePath();

                FileReader fr = null;
                BufferedReader br = null;

                try {
                    fr = new FileReader(fileName);
                    br = new BufferedReader(fr);
                    String sCurrentLine;
                    while ((sCurrentLine = br.readLine()) != null) {
                        contents += sCurrentLine.trim();
                    }
                    if (contents.equals("")) {
                        contents = "empty";
                    }

                    return contents;
                } catch (Exception e) {
                    e.printStackTrace();

                    if (contents.equals("")) {
                        contents = "empty";
                    }
                    return contents;
                } finally {
                    if (fr != null) {
                        try {
                            fr.close();
                        } catch (Exception e) {
                            e.printStackTrace();
                        }
                    }

                    if (br != null) {
                        try {
                            br.close();
                        } catch (Exception e) {
                            e.printStackTrace();
                        }
                    }

                    if (map.containsKey(name)) {
                        map.remove(name);
                    }

                    map.put(name, contents);
                }
            }
        }
    } catch (Exception e) {
        e.printStackTrace();

        if (contents.equals("")) {
            contents = "empty";
        }
        return contents;
    }
    return contents;
}

person t.smith.htc    schedule 09.07.2012    source источник
comment
Я просто не верю, что foo.indexOf("empty") когда-нибудь вернет foo.length() - 1 для непустой строки. Это означало бы, что String.indexOf очень сломан. Я не верю, что ConcurrentHashMap или String сломаны - я сильно подозреваю, что ваш код где-то сломан.   -  person Jon Skeet    schedule 09.07.2012
comment
Можете ли вы показать код getDataFromFileSystem(name);?   -  person assylias    schedule 09.07.2012
comment
это фактический метод getData(), или вы переработали его, чтобы опубликовать здесь?   -  person jtahlborn    schedule 09.07.2012
comment
Метод actual getData() имеет дополнительную регистрацию, которая была удалена для этого сообщения.   -  person t.smith.htc    schedule 09.07.2012
comment
Я определю функцию getDataFromFileSystem(name); в посте ниже...   -  person t.smith.htc    schedule 09.07.2012
comment
В getDataFromFileSystem есть несколько проверок для contents.equals(""), но это никогда не может быть правдой, потому что String contents = "empty";.   -  person Miserable Variable    schedule 10.07.2012
comment
Я добавил, что в случае, если содержимое файла пусто и копируется в содержимое переменной.   -  person t.smith.htc    schedule 10.07.2012


Ответы (3)


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

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

  • Поток 1 читает эту строку в методе getData:

    if (map.containsKey(name)) // (1)
    
  • результат ложный и поток 1 переходит к

    reply = getDataFromFileSystem(name); // (2)
    
  • в getDataFromFileSystem у вас есть следующий код:

    if (map.containsKey(name)) { // (3)
        map.remove(name);  // (4)
    }
    map.put(name, contents); // (5)
    
  • представьте, что другой поток (поток 2) достигает (1), в то время как поток 1 находится между (4) и (5): имя отсутствует на карте, поэтому поток 2 снова переходит к (2)

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

В нынешнем виде я не могу найти объяснение описанному вами сценарию, если только вы не вызовете reply = map.get(name) более одного раза в своих тестах, и в этом случае вполне возможно, что 2 вызова не возвращают один и тот же результат.

person assylias    schedule 10.07.2012
comment
Спасибо за помощь assylias. Я обновлю свою функцию, чтобы обеспечить потокобезопасность, а затем отслеживаю поведение. - person t.smith.htc; 11.07.2012

Во-первых, даже не думайте, что в ConcurrentHashMap есть ошибка. Ошибки JDK очень редки, и даже интерес к этой идее отвлечет вас от правильной отладки кода.

Я думаю, что ваша ошибка заключается в следующем. Поскольку вы используете contains("empty"), что произойдет, если в строке файла будет слово "empty"? Разве это не испортит дело?

Вместо использования contains("empty") я бы использовал ==. Сделайте «пустой» private static final String, тогда вы сможете использовать для него равенство.

private final static String EMPTY_STRING_REFERENCE = "empty";
...
if (reply != null && reply != EMPTY_STRING_REFERENCE) {
    return reply;
}
...
String contents = EMPTY_STRING_REFERENCE;
...
// really this should be if (contents.isEmpty())
if (contents.equals("")) {
    contents = EMPTY_STRING_REFERENCE;
}

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

Вот некоторые другие моменты:

  • В общем, всякий раз, когда вы используете один и тот же String в нескольких местах вашей программы, его следует подтягивать к полю static final. Java, вероятно, все равно сделает это за вас, но также делает код намного чище.
  • @assylias точно знает об условиях гонки, когда вы делаете 2 звонка ConcurrentHashMap. Например, вместо того, чтобы делать:

    if (map.containsKey(name)) {
        reply = map.get(name);
    } else {
    

    Вы должны сделать следующее, поэтому вы делаете только один.

    reply = map.get(name);
    if (reply == null) {
    
  • В вашем коде вы делаете это:

    if (map.containsKey(name)) {
         map.remove(name);
    }
    map.put(name, contents);
    

    Это следует переписать следующим образом. Нет необходимости удалять перед установкой, которая вводит условия гонки, как упоминал @assylias.

    map.put(name, contents);
    
  • #P10# <блочная цитата> #P11# #P12#
person Gray    schedule 10.07.2012
comment
Я бы не рекомендовал использовать == для EMPTY_STRING, поскольку инструменты анализа кода сообщат об этом как об ошибке, и следующий разработчик поддержки может ошибиться, чтобы исправить это. Глядя на реализацию OP, нет очевидной причины не возвращать просто пустую строку, если ничего не было прочитано. - person Arne; 10.07.2012
comment
Я не уверен, что инструмент анализа кода сообщит об этом, но может быть. Но следующий пункт разработчика — хороший. Я переименовал его в _REFERENCE. - person Gray; 10.07.2012
comment
Хорошая идея с точки зрения пустой строки, хотя мне больше нравится эталонный идентификатор. - person Gray; 10.07.2012

Во-первых, использование ConcurrentHashMap не защищает вас, если вы последовательно вызываете его методы из нескольких потоков. Если вы вызовете containsKey и get после этого, а другой поток вызовет remove между ними, вы получите нулевой результат. Обязательно вызывайте только get и проверяйте значение null вместо containsKey/get. Это также лучше с точки зрения производительности, потому что оба метода имеют почти одинаковую стоимость.

Во-вторых, странный результат вызова indexOf либо связан с ошибкой программирования, либо указывает на повреждение памяти. Есть ли в вашем приложении собственный код? Что ты делаешь в getDataFromFileSystem? Я наблюдал повреждение памяти при использовании объектов FileChannel из нескольких потоков.

person Arne    schedule 09.07.2012
comment
У меня нет нативных вызовов в моем приложении. getDataFromFileSystem теперь определено в моем исходном сообщении. Функция просто читает файл, используя BufferedFileReader. - person t.smith.htc; 10.07.2012
comment
Кроме того, я изменил способ доступа к карте — вместо вызова containsKey с последующим get я вызываю get, а затем проверяю значение null. Спасибо за чаевые :) - person t.smith.htc; 10.07.2012