Remove use of File{Input,Ouput}Stream#2649
Conversation
These two classes have a finalizer in Java 12, hence they are an unnecessary burden for GC. Use: - Files.newInputStream() instead of FileInputStream - Files.newBufferedReader() when we end up converting to a Writer - Files.readAllBytes() if we end up reading the stream - Files.copy() if we are copying the stream to another stream or file - Files.writeString() when we are emitting just a plain string - Files.newOutputStream() instead of FileOutputStream - Files.newBufferedReader() when we end up conerting to a Writer Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
| OutputStream os = new FileOutputStream(tmpFile)) { | ||
| StreamUtils.copy(is, os); | ||
| try (var is = urlObj.openStream()) { | ||
| Files.copy(is, tmpFile.toPath()); |
There was a problem hiding this comment.
I think this will always throws FileAlreadyExistsException because Files.creqteTempFile() creates the file before the copy is attempted. REPLACE_EXISTING is required here.
| { | ||
| StreamUtils.copy(is, os); | ||
| try (var is = new URL(url).openStream()) { | ||
| Files.copy(is, tmpFile.toPath()); |
There was a problem hiding this comment.
I think this will always throws FileAlreadyExistsException because Files.creqteTempFile() creates the file before the copy is attempted. REPLACE_EXISTING is required here.
| ) { | ||
| StreamUtils.copy(is, fop); | ||
| } | ||
| Files.copy(is, file.toPath()); |
There was a problem hiding this comment.
Without REPLACE_EXISTING, it throws when override=true and the config file already exists. The old FileOutputStream silently overwrote. With Files.copy() will always fail the override path. The same happens in InstallCommand and ConfigMBeanImpl.
| FileOutputStream out = new FileOutputStream(dest); | ||
| StreamUtils.copy(is, out); | ||
| out.close(); | ||
| Files.copy(is, dest.toPath()); |
There was a problem hiding this comment.
I think REPLACE_EXISTING is required here. On KAR re-install or upgrade, previously extracted files exist, causing FileAlreadyExistsException and leaving the KAR partially extracted.
| return new String(outputStream.toByteArray(), StandardCharsets.UTF_8); | ||
| } | ||
| private static String loadConfiguration(final URL url) throws IOException { | ||
| return new String(url.openStream().readAllBytes(), StandardCharsets.UTF_8); |
There was a problem hiding this comment.
Here, we potentially leaks the URL stream if readAllBytes() throws mid-read. Previously, we used a proper try-with-resources approach.
2 participants
These two classes have a finalizer in Java 12, hence they are an
unnecessary burden for GC.
Use:
Signed-off-by: Robert Varga robert.varga@pantheon.tech