Use try-with-resources in HttpTunnelPayload #11779
Conversation
spring-projects-issues
added
the
status: waiting-for-triage
philwebb
added
type: enhancement
|
Thanks. I'm not sure if all of these are valid or not so will review before merging. |
|
@rajadilipkolli @philwebb thank you for attention to this issues! But this is false positive static code analysis warning. Possible resource leak already fixed in PR 11624 Need to redesign this classes to avoid this sonarcloud warnings. Log4J2LoggingSystem.java is not applicable - stream should be in open state after return of ConfigurationSource JarWriter.java - output stream created in construnctor and used in another object methods SocketTargetServerConnection.java - network channel should be in open state after method return |
|
HttpTunnelPayload.java looks like real fix (if exception occurred) |
| if (launchScript != null) { | ||
| fileOutputStream.write(launchScript.toByteArray()); | ||
| setExecutableFilePermission(file); | ||
| try (FileOutputStream fileOutputStream = new FileOutputStream(file)) { |
There was a problem hiding this comment.
This will close fileOutputStream before it can be written to via this.jarOutput. It's eventually closed when JarWriter is closed so this is change isn't needed. I'll address this while merging.
| .createServerLocatorWithoutHA(transportConfiguration); | ||
| return factoryClass.getConstructor(ServerLocator.class) | ||
| .newInstance(serviceLocator); | ||
| try (ServerLocator serviceLocator = ActiveMQClient |
There was a problem hiding this comment.
This will close the ServerLocator before the connection factory can use it. It is eventually closed when the connection factory is closed so this change is unnecessary. I'll address this when merging.
| this.serverThread = new ServerThread(serverSocketChannel); | ||
| this.serverThread.start(); | ||
| return port; | ||
| try (ServerSocketChannel serverSocketChannel = ServerSocketChannel.open()) { |
There was a problem hiding this comment.
This will close the ServerSocketChannel before it can be used by the ServerThread. The channel is eventually closed when the thread is closed so this change is unnecessary. I'll address this when merging.
| SocketChannel channel = SocketChannel.open(address); | ||
| channel.socket().setSoTimeout(socketTimeout); | ||
| return new TimeoutAwareChannel(channel); | ||
| try (SocketChannel channel = SocketChannel.open(address)) { |
There was a problem hiding this comment.
This will close the SocketChannel before it can be used by TimeoutAwareChannel. It's eventually closed when the TimeoutAwareChannel is closed so this is change isn't needed. I'll address this while merging.
| InputStream stream = url.openStream(); | ||
| if (FILE_PROTOCOL.equals(url.getProtocol())) { | ||
| return new ConfigurationSource(stream, ResourceUtils.getFile(url)); | ||
| try (InputStream stream = url.openStream()) { |
There was a problem hiding this comment.
This will close the InputStream before it can be retrieved from the ConfigurationSource and used. Log4j2 makes it the responsibility of the user of the ConfigurationSource to retrieve the InputStream and close it so this change is unnecessary.
wilkinsona
changed the title
wilkinsona
added
type: task
Resources are not closed, hence closing using try-with-resources block