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());
    }
}

}

4

3 回答 3

3

在我看来，这不是线程安全的。我对线程安全的理解意味着两个线程可以同时调用此类的同一个实例上的方法，并且该类将按预期运行。这对于这个类是不正确的。

考虑一下：

线程 1 调用readFiles()导致此行被调用：

lines = new ArrayList<String>();

readFiles()现在想象第二个线程也在第一个线程完成之前调用。再次调用此行：

lines = new ArrayList<String>();

您现在已经用第二个变量覆盖了第一个变量。

第一个线程将调用lines.addAll(fileLines)它实际上将使用第二个lines变量。这可能不是你想要的。

最简单的解决方法是将synchronized关键字添加到方法签名中。这当然会减慢它的速度，因为第二个调用将等待第一个调用完成。

您应该readFiles()实例化它自己List并返回它自己的实例。然后删除私有级别的行变量。

于 2012-06-19T19:14:20.793 回答
2

如上所述，读取的代码本身是线程安全的，但迭代器方法不应该是公共的。

问题是您实现了 Iterable 接口。
我认为这是一个糟糕的设计。

你不应该在这里实现Iterable，而是有一个方法返回一个String对象的Iterable，只有在你完成读取文件后才会返回一个Iterable对象。
这样，您可以拥有读取文件并为其内容提供可迭代的功能。
我建议如下：

A. 有一个名为 MyFileReader 的类
B. 有你在上面定义的字段来保存行。C. 有一个CountdownLatch对象 - 调用这个字段锁。让它初始化为 1。
D. 一旦文件被读取，您的 readFiles 方法应该执行 countDown。
E. 定义一个 getIterator 方法，它将为 lines 字段创建一个迭代器，但在创建之前，它将调用 lock.await() - 这意味着该方法将等待文件被完全读取。

我希望它更清楚。我真的应该不明白如何嵌入代码。我还没明白:(

于 2012-06-19T18:22:50.553 回答
1

即使文件读取本身是线程安全的，但读取的数据却不是。

您的iterator()方法是公开的，当一个线程正在读取文件时，另一个线程可以调用该iterator()方法并获取 ConcurrentModificationException。（因为 ArrayList 是快速失败的）。

于 2012-06-19T18:15:29.697 回答