Feature Request: Trace down unclosed directory 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

[Clasmir]

I am evaluating YourKit and at the moment using the Streams probe. We've found what might be a bug within the probe itself.

The following code (specifically the 'new WindowsDirectoryStream(file, null).close() ) shows streams being opened but never closed:


Is this a problem with the streams probe? We are executing within a wildfly 16 java 11 environment.

Code: Select all

WindowsFileSystemProvider.java:340 sun.nio.fs.WindowsDirectoryStream.<init>(WindowsPath, DirectoryStream$Filter) 2591 0

[Clasmir]

For some reason I could not post this in a new thread as it flagged the material as spam

https://www.dropbox.com/s/t48p7q5chbghako/YourKitPossibleStreamsLeak.PNG?dl=0

[Anton Katilin]

Hi,

Thank you for pointing to this issue.

Please see the source code of method checkReadAccess() in sun.nio.fs.WindowsFileSystemProvider. It uses "new WindowsDirectoryStream(file, null).close()" to generate proper IOException.

The probe is right: it registers directory stream creation attempt, i.e. a call of the stream constructor, which fails with exception, and hence close() is not invoked.

Please go to "Events" tab, switch to "Events by table", select the problematic stream which should have 0 in "Close count" column, then in the bottom table "Open" you'll see the exception in "Exception" column.

By the way, the inspection "Not closed directory streams" does not report this case, because the stream creation failed on exception, and therefore there is nothing to close.

Conclusion: the probe and the inspection on the probe are correct, although the fact that failed creation attempts are also recorded may cause a confusion.

Best regards,
Anton

[Clasmir]

Thank you for the reply.

Let's see if I understand.

* So the code attempts to open a directory but fails generating an exception. Likely because it isn't a directory, or any number of permission issues.
* The probe still counts this as an stream even though it isn't actually there??
* The close isn't invoked because the exception occurred before it could be reached.

Is that correct?

If it is, how can we say that's expected behavior in the final tally of open and closed streams? The stream was never opened?

Is there a way I can filter out these streams that we aborted (before they became real) because of an exception? So that when i try to hunt for real problems, I can do so?

Thank you for your time.

DK

[Anton Katilin]

Hi,

* So the code attempts to open a directory but fails generating an exception. Likely because it isn't a directory, or any number of permission issues.

Yes.

* The probe still counts this as an stream even though it isn't actually there??

It counts it as an attempt to open a stream, and indicates that the attempt has failed by recording the exception.

* The close isn't invoked because the exception occurred before it could be reached.

Yes.

Is that correct?

Yes, all is correct.

If it is, how can we say that's expected behavior in the final tally of open and closed streams? The stream was never opened?

Good question. Currently, "Open" events contain both successful and failed attempts. Failed ones have non-empty value in "Exception" column.

Is there a way I can filter out these streams that we aborted (before they became real) because of an exception? So that when i try to hunt for real problems, I can do so?

As I wrote, the inspection "Not closed directory streams" skips failed streams and never reports them as a problem. So if the inspections finds no problem, there is no problem.

There is no dedicated filter for that in "Events", and likely we should add one.

Best regards,
Anton

[Clasmir]

Thank you.

Where do I find: "the inspection "Not closed directory streams" " ?

That would indeed solve the problem, I can't seem to find it.

[Clasmir]

I knew as soon as I wrote that, I'd find it. I was looking within the memory tab, didn't even realize there is an inspections complete tab.... /sigh

[Anton Katilin]

Yes, event inspections are in the top level "Inspections" tab only, they are not present in memory tabs:
https://www.yourkit.com/docs/java/help/ ... ctions.jsp

Memory inspections are available in the top-level "Inspections" tab as well as in local "Inspections" views in memory tabs, and respect the tab's object set.
https://www.yourkit.com/docs/java/help/ ... ns_mem.jsp