[JAVA] squid:S2095 Resources should be closed - FP - marks InputStream object passed to Reader constructor

[Adam Gabryś]

Hello,
Java Plugin 4.2 reports a False Positive for SQUID:S2095 (https://sonarqube.com/coding_rules#rule_key=squid%3AS2095) on code which creates a Reader, which takes an InputStream as constructor argument. The reader is closed in finally block. I know that SonarQube (probably) does not recognize custom close methods, but I remember there was a topic about add them. What is the status?

Link to file: https://github.com/gabrysbiz/lesscss-compiler/blob/develop/src/main/java/biz/gabrys/lesscss/compiler/LessCompilerImpl.java#L159

Regards,
Adam Gabryś

[Paul Wagland]

Being able to specify that a method closes a resource would be really convenient for our code base as well, we use a similar pattern for closing JDBC Connections.

[Nicolas Peru]

Hi Paul and Adam, 

Issue is not what you think here AFAI can tell : you both assume that the close method is not detected, but if it was the case then the reader would be highlighted. And as per https://jira.sonarsource.com/browse/SONARJAVA-1688 detecting closing method should (and do) work.

What is happening here is that the constructor call "new InputStreamReader" might end up throwing an exception (very very unlikely and probably nearly impossible in this _very_ case I admit).

This would cause the reader to be null and SequenceInputStream to be left open hence the issue that is raised.

So there are two problems at stake here : first, reporting: it is absolutely unclear from a user perspective why the engine raised such an issue. This is something we know we have to improve (a lot) for all rules relying on symbolic execution engine. 

Second, how to detect that in practice nothing really dangerous is at risk here ? I honestly don't have a clear idea on that because you can always produce a case where there might be something thrown from the constructor call (OutOfMemoryError, StackOverflowError, etc... ) so things are not that simple to simply exclude that case. 

+Paul Wagland regarding your suggestion for a rule parameter : that would be definitely a no given the state of things (that would lead to a maintainance hell) and we plan to solve this issue with cross procedural symbolic execution.

Cheers, 

Le jeu. 6 oct. 2016 à 13:52, Paul Wagland <pwag...@gmail.com> a écrit :

Being able to specify that a method closes a resource would be really convenient for our code base as well, we use a similar pattern for closing JDBC Connections.

--
You received this message because you are subscribed to the Google Groups "SonarQube" group.
To unsubscribe from this group and stop receiving emails from it, send an email to sonarqube+...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/sonarqube/8728b7a4-0996-4319-a3f9-ba12602ed4d4%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

--

Nicolas PERU | SonarSource

Senior Developer
http://sonarsource.com

https://twitter.com/benzonico