Feature Request: Trace down unclosed direcotry streams

[Thorsten Goetzke]

Hello,

Java 8 allows you to do this:

Code: Select all

   Files.list(Paths.get("")).forEach(System.out::println);  //Do not copy& paste this

It's not obvious that the code above has a memoryleak: The jvm will open a native directory descriptor pointing to the current directory, but won't close it. Even the GC won't save you.
The correct code to use would be

Code: Select all

   try (final Stream<Path> list = Files.list(Paths.get(""))){
            list.forEach(System.out::println);
 }

I would appreciate it if Yourkit could help to trace down these Mistakes in a similar fashion as does with unclosed FileInputStreams.

[Anton Katilin]

Hello Thorsten,

Thank you for the suggestion.

It seems that technically this means we should track instances of classes implementing java.nio.file.DirectoryStream (e.g. sun.nio.fs.WindowsDirectoryStream, com.sun.nio.zipfs.ZipDirectoryStream, etc.) for which close() is never called.

We might do this with a new probe keeping a list of created streams and removing from the list those of them for which close() is called. I think you agree that it is not enough if the profiler just tells you that such objects exist, it should also tell where in the code they are created, i.e. to record the stack trace. Because it is yet unknown at the moment of the object creation whether it will be explicitly released with close() in the future, we'll have to record a stack trace for each created stream object. In some cases this might probably add significant overhead.

Anyway, we may try adding such probe. Will you be interested in early testing it?

By the way, I see an alternate approach: while finding such cases in runtime may not be free from the performance point of view, it instead should be just trivial (of course, given all existing capabilities) for an IDE performing static code analysis to immediately find all wrong cases.

Which IDE do you use? IntelliJ IDEA has inspection "AutoCloseable used without 'try'-with-resources"; I just tried it and it doesn't seem to find unwrapped Files.list() in the current version, but we could send the vendor a feature request asking to support them too. I'm not sure about Eclipse but they likely have something similar.

Best regards,
Anton

[Thorsten Goetzke]

Hello Anton,

Thanks for your reply. I would love to have a look at your probe.
I tried to use "AutoCloseable used without 'try'-with-resources";, it doesn't add a warning to the given Code.
I guess the problem is that the inspection just evaluates the declared Streamtype (which is just java.util.Stream and not DirectoryStream).
Obviously "almost" none of the "normal" java.util.Stream needs to be closed, so the inspections seems somewhat useless ..

[Anton Katilin]

Hello Thorsten,

To try the proposed probe please do the following:

1. Download streams.jar which contains the new probe:
https://www.yourkit.com/download/tmp/20 ... treams.jar

2. Profile with the following agent startup options:

probebootclasspath=<full path to>/streams.jar,probe_on=.DirectoryStreams

(How to specify the options: https://www.yourkit.com/docs/java/help/ ... ptions.jsp )

3. In the "Events" tab see the new table "Not Closed Directory Streams".

Best regards,
Anton

[Thorsten Goetzke]

Hello,

The Probe does correctly report unclosed Streams and removes closed Streams accordingly. Personally I find it a bit inconsistent compared to the way Yourkit analyses FileInputstreams. The latter has the probes only reporting the lifecycle, while finding unclosed FileInputStreams has to be done in the tab "Inspections".
But I have to admit seeing unclosed streams directly without having to push buttons is nice.

Best Regards,
Thorsten Goetzke

[Anton Katilin]

Hello Thorsten

Thank you for the feedback.

Indeed, this proposed probe directly shows non-closed streams unlike other probes keeping all event detail and requiring an inspection to easily find non-closed resources. This probe was a quick experiment, a proof of concept. If we keep this feature, we'll likely change it to the common scheme. On the other hand, the common scheme would have slightly higher overhead which is justified for file streams because not only the close invocation status is remembered but also read-write operations. I don't think it would make sense to record more detail for directory streams in general case, like particular files listed, as they are likely useless in most cases unless you perform some very special debugging.

Best regards,
Anton

[Anton Katilin]

Hello Thorsten

FYI:

In the next 2019.8 EAP build the built-in probe DirectoryStreams will be reworked to match other probes' behavior, and the new inspection "Not closed directory streams" will be introduced.

Best regards,
Anton