2012-06-19 2 views
2

Может ли кто-нибудь просмотреть этот код, чтобы убедиться, что он потокобезопасен?Этот класс читает файловую цепочку безопасно?

public class FileLinesSorter { 
private CountDownLatch startSignal = new CountDownLatch(1); 

/** 
* The lines 
*/ 
private List<String> lines; 

/** 
* Read files from the file paths 
* @param filePaths the list of file paths 
* @throws IOException on I/O error 
*/ 
public void readFiles(String[] filePaths) throws IOException { 
    lines = new ArrayList<String>(); 
    for (String filePath : filePaths) { 
     File file = new File(filePath); 
     if (!file.exists()) { 
      // File does not exist. Log it. 
      continue; 
     } 
     List<String> fileLines = readFile(file); 
     lines.addAll(fileLines); 
    } 
    if (!lines.isEmpty()) { 
     Collections.sort(lines); 
    } 
    startSignal.countDown(); 
} 

/** 
* Read a single file 
* @param file the file 
* @return the file content 
* @throws IOException on I/O error 
*/ 
private List<String> readFile(File file) throws IOException { 
    List<String> contents = new ArrayList<String>(); 
    BufferedReader reader = null; 
    FileReader fileReader = null; 
    try { 
     fileReader = new FileReader(file.getAbsolutePath()); 
     reader = new BufferedReader(fileReader); 
     String line = ""; 
     while ((line = reader.readLine()) != null) { 
      if (line.isEmpty()) { 
       continue; 
      } 
      contents.add(line); 
     } 
    } catch (FileNotFoundException e) { 
     throw e; 
    } catch (IOException e) { 
     throw e; 
    } finally { 
     if (fileReader != null) { 
      fileReader.close(); 
     } 
     if (reader != null) { 
      reader.close(); 
     } 
    } 
    return contents; 
} 


public Iterator<String> getIterator() { 
    try { 
     startSignal.await(); 
    } catch (InterruptedException e) { 
     e.printStackTrace(); 
    } 
    return lines.iterator(); 
} 

public static void main(String[] args) throws Exception { 
    String[] filePaths = {"C:\\Works\\files\\example-1 copy.txt", "C:\\Works\\files\\example-2 copy.txt"}; 
    FileLinesSorter sorter = new FileLinesSorter(); 
    sorter.readFiles(filePaths); 
    Iterator<String> lines = sorter.getIterator(); 
    while (lines.hasNext()) { 
     System.out.println(lines.next()); 
    } 
} 

}

ответ

3

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

Рассмотрит это:.

Thread 1 вызывает readFiles() что приводит к этой линии называют:

lines = new ArrayList<String>(); 

Теперь представьте второй поток также вызывает readFiles() до первая резьба заканчивается. Эта строка вызывается снова:

lines = new ArrayList<String>(); 

Теперь вы переписали первую переменную со второй переменной.

Первый поток будет называть lines.addAll(fileLines), который фактически будет использовать вторую переменную lines. Это, вероятно, не то, что вы хотите.

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

Вы должны сделать readFiles() экземпляр своего собственного List и вернуть его собственный экземпляр. Затем удалите переменную линии частного уровня.

1

Даже если файл читает сам является поточно, данные для чтения нет.

Ваш метод iterator() является общедоступным, и хотя поток читает файл, другой поток может вызвать метод iterator() и получить ConcurrentModificationException. (Поскольку ArrayList не работает быстро).

+0

означает, что клиент (кто бы это ни называл этот класс) должен заботиться о потокобезопасности вместо FileLinesSorter? – user826323

+0

необязательно, вы можете использовать параллельную версию списка, например ConcurrentLinkedQueue –

2

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

Проблема в том, что вы реализуете интерфейс Iterable.
Я думаю, что это плохой дизайн.

Здесь вы не должны реализовывать Iterable, а есть метод, возвращающий объект Iterable of String, который вернет объект Iterable только после того, как вы закончите чтение файла.
Таким образом, вы можете иметь функциональность чтения файла и обеспечивать его итерабельность его содержимого.
Я предлагаю следующее: A. У вас есть класс под названием MyFileReader
B. Укажите поле, указанное выше для хранения строк. C. Имейте объект CountdownLatc h - вызовите это полевое блокирование. Инициализировать его со значением 1.
D. Ваш метод readFiles должен выполнять countDown после чтения файла.
E. Определите метод getIterator, который создаст итератор для поля линий, но до создания он вызовет lock.await() - это означает, что этот метод будет ждать, пока файл не будет полностью прочитан.

Надеюсь, что это более понятно. Мне действительно нужно разобраться, как вставлять код. Я еще не понял это :(

+0

Не могли бы вы изменить ее, чтобы я мог лучше понять? – user826323

+0

Я думаю, что он говорит, что не реализует Iterable. Просто используйте метод 'getLines()', который возвращает строки, или метод 'getLinesIterator()', который возвращает 'lines.iterator()' и _only делает это после полного чтения файла. – user949300

+0

Точно, я новичок в stackoverflow, и я не знаю, как вставлять код. Я попытаюсь изменить это. –

Смежные вопросы