Hello @core-libs-dev <[email protected]>,

I was writing a simple script today -- find how many Java files are in each
of my top level folders of my current directory.

So, I ran the following code.

Files
    list(Path.of("."))
    filter(Files::isDirectory)
    collect

        Collectors.groupingBy(
        Function.<Path>identity(),
        Collectors
            .flatMapping
            (
                (final Path root) ->
                {
                    try (final Stream<Path> streamWalker = Files.walk(root))
                    {
                        return
                            streamWalker
                                .filter(Files::isRegularFile)
                                .filter(file ->
file.toString().endsWith(".java"))
                                ;
                    }
                    catch (final Exception exception)
                    {
                        throw new RuntimeException(exception);
                    }
                },
                Collectors.counting()
            )
        )
        ;

Now, there is a not-so-obvious bug -- when using a flatMapping Collector,
no need for try-with-resources.

The above code threw the below stack trace.

|  Exception java.lang.IllegalStateException
|        at FileTreeIterator.hasNext (FileTreeIterator.java:100)
|        at Iterator.forEachRemaining (Iterator.java:132)
|        at Spliterators$IteratorSpliterator.forEachRemaining
(Spliterators.java:1939)
|        at AbstractPipeline.copyInto (AbstractPipeline.java:570)
|        at AbstractPipeline.wrapAndCopyInto (AbstractPipeline.java:560)
|        at ForEachOps$ForEachOp.evaluateSequential (ForEachOps.java:153)
|        at ForEachOps$ForEachOp$OfRef.evaluateSequential
(ForEachOps.java:176)
|        at AbstractPipeline.evaluate (AbstractPipeline.java:265)
|        at ReferencePipeline.forEach (ReferencePipeline.java:632)
|        at Collectors.lambda$flatMapping$0 (Collectors.java:483)
|        at Collectors.lambda$groupingBy$0 (Collectors.java:1113)
|        at ReduceOps$3ReducingSink.accept (ReduceOps.java:169)
|        at ReferencePipeline$2$1.accept (ReferencePipeline.java:197)
|        at Iterator.forEachRemaining (Iterator.java:133)
|        at Spliterators$IteratorSpliterator.forEachRemaining
(Spliterators.java:1939)
|        at AbstractPipeline.copyInto (AbstractPipeline.java:570)
|        at AbstractPipeline.wrapAndCopyInto (AbstractPipeline.java:560)
|        at ReduceOps$ReduceOp.evaluateSequential (ReduceOps.java:921)
|        at AbstractPipeline.evaluate (AbstractPipeline.java:265)
|        at ReferencePipeline.collect (ReferencePipeline.java:723)
|        at (#4:5)


Not obvious what is wrong here.

Now, the exception message is being thrown by the hasNext() method of the
FileTreeIterator class. After looking at the source code of that class,
it's clear what the problem is.

https://github.com/openjdk/jdk/blob/jdk-26%2B25/src/java.base/share/classes/java/nio/file/FileTreeIterator.java#L98

It throws an exception because the FileTreeWalker contained within the
FileTreeIterator is closed (presumably by the try-with-resources). However,
the thrown exception message is empty. This exact same check-then-throw
logic is duplicated all over FileTreeIterator.

I propose that all such instances of check-then-throw be provided with
exception messages along the lines of "Cannot check for more elements
because the file walker is already closed!". I am not picky on the wording.
Just something to inform me that the issue is something is closed, as
opposed to just a generic IllegalStateException. I would even accept an
empty message body if the exception type communicated the issue.

Thank you for your time.
David Alayachew

Reply via email to