CODEWITHSUNDEEP
javamultithreadingsearch
user1653773
user1653773

Reputation: 373

Traversing and enumerating a directory with multi-threads

I am running a thread to traverse my local directory (no sub directory) and as soon as I am getting a text file, I am starting a new thread which will search a word in that file.

What is wrong in the below code?

Searching and traversing are working fine, separately. But when I am putting it together, some thing is going wrong, it is skipping some files (Not exactly, due to multithreading object sunchronization is not happening properly).

Please help me out.

Traverse.java 

    public void executeTraversing() {
            Path dir = null;
            if(dirPath.startsWith("file://")) {
                    dir = Paths.get(URI.create(dirPath));
            } else {
                    dir = Paths.get(dirPath);
            }
            listFiles(dir);
    }

    private synchronized void listFiles(Path dir) {
            ExecutorService executor = Executors.newFixedThreadPool(1);
            try (DirectoryStream<Path> stream = Files.newDirectoryStream(dir)) {
                    for (Path file : stream) {
                            if (Files.isDirectory(file)) {
                                    listFiles(file);
                            } else {
                                    search.setFileNameToSearch(file);
                                    executor.submit(search);
                            }
                    }
            } catch (IOException | DirectoryIteratorException x) {
                    // IOException can never be thrown by the iteration.
                    // In this snippet, it can only be thrown by
                    // newDirectoryStream.
                    System.err.println(x);
            }
    }

Search.java

    /**
     * @param wordToSearch
     */
    public Search(String wordToSearch) {
            super();
            this.wordToSearch = wordToSearch;
    }

    public void run() {
            this.search();    
    }

    private synchronized void search() {
            counter = 0;

            Charset charset = Charset.defaultCharset();
            try (BufferedReader reader = Files.newBufferedReader(fileNameToSearch.toAbsolutePath(), charset)) {
                    // do you have permission to read this directory?
                    if (Files.isReadable(fileNameToSearch)) {
                            String line = null;
                            while ((line = reader.readLine()) != null) {
                                    counter++;
                                    //System.out.println(wordToSearch +" "+ fileNameToSearch);

                                    if (line.contains(wordToSearch)) {
                                            System.out.println("Word '" + wordToSearch
                                                            + "' found at "
                                                            + counter
                                                            + " in "
                                                            + fileNameToSearch);
                                    }
                            }
                    } else {
                            System.out.println(fileNameToSearch
                                            + " is not readable.");
                    }

            } catch (IOException x) {
                System.err.format("IOException: %s%n", x);
            }

    }

Upvotes: 2

Views: 1166

Answers (2)

Guillaume
Guillaume

Reputation: 14656

You are creating the ExecutorService inside your listFiles method, this is probably not a good idea: because of that you're probably creating too many threads.

On top of that you're not monitoring the state of all these ExecutorServices, some of them might not be started when you application stops

Instead you should create the ExecutorService only once, before starting the recursion. When the recursion is over, call shutdown() on your ExecutorService to wait for all tasks completion

Furthermore you are reusing a Search object and passing it to mutliple tasks while modifying it, you should create a Search for each file you're processing

Upvotes: 2

radai
radai

Reputation: 24192

this Search instance that you keep reusing here:

search.setFileNameToSearch(file);
executor.submit(search);

while its actual search() method is synchronized, it appears like by the time it actually gets to searching something setFileNameToSearch() would have been called several times, which would explain the skipping.

create a new instance of Search each time, then you wouldnt need to sync the actual search() function.

Upvotes: 4

Related Questions