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

私の意見では、これはスレッドセーフではありません。スレッドセーフについての私の理解は、2 つのスレッドがこのクラスの同じインスタンスでメソッドを同時に呼び出すことができ、クラスが期待どおりに動作することを意味します。これは、このクラスには当てはまりません。

このことを考慮：

スレッド 1 呼び出しreadFiles()により、次の行が呼び出されます。

lines = new ArrayList<String>();

readFiles()ここで、最初のスレッドが終了する前に2 番目のスレッドも呼び出すとします。この行が再度呼び出されます。

lines = new ArrayList<String>();

これで、最初の変数が 2 番目の変数で上書きされました。

最初のスレッドが呼び出さlines.addAll(fileLines)れ、実際には 2 番目のlines変数が使用されます。これはおそらくあなたが望むものではありません。

最も簡単な修正方法は、synchronizedキーワードをメソッド シグネチャに追加することです。2 番目の呼び出しは最初の呼び出しが終了するまで待機するため、これはもちろん速度を低下させます。

readFiles()インスタンス化を独自のListものにして、独自のインスタンスを返す必要があります。次に、プライベート レベルの回線変数を削除します。

于 2012-06-19T19:14:20.793 に答える
2

上で提案したように、読み取りコード自体はスレッド セーフですが、イテレータ メソッドは公開しないでください。

問題は、 Iterable インターフェースを実装していることです。
これはデザインが悪いと思います。

ここで Iterable を実装するのではなく、ファイルの読み取りが完了した後にのみ Iterable オブジェクトを返す String オブジェクトの Iterable を返すメソッドを用意する必要があります。
このようにして、ファイルを読み取り、そのコンテンツに iterable を提供する機能を持つことができます。
次のことをお勧めします。

A. MyFileReader というクラスを作成
します。 B. 行を保持するために上で定義したフィールドを作成します。C. CountdownLatchオブジェクトを用意する - このフィールド ロックを呼び出します。値 1 で初期化します。
D. ファイルが読み取られたら、readFiles メソッドで countDown を実行する必要があります。
E. lines フィールドの反復子を作成する getIterator メソッドを定義しますが、作成前に lock.await() を呼び出します。これは、このメソッドがファイルが完全に読み取られるまで待機することを意味します。

より明確になることを願っています。コードを埋め込む方法を本当に理解する必要があります。まだわかりません:(

于 2012-06-19T18:22:50.553 に答える
1

ファイルの読み取り自体はスレッドセーフですが、読み取ったデータはそうではありません。

メソッドiterator()はパブリックであり、スレッドがファイルを読み取っている間に、別のスレッドがiterator()メソッドを呼び出して ConcurrentModificationException を取得する可能性があります (ArrayList はフェイルファストであるため)。

于 2012-06-19T18:15:29.697 に答える