[ https://issues.apache.org/jira/browse/ZOOKEEPER-4246?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
ASF GitHub Bot updated ZOOKEEPER-4246: -------------------------------------- Labels: pull-request-available (was: ) > Resource leaks in > org.apache.zookeeper.server.persistence.SnapStream#getInputStream and > #getOutputStream > -------------------------------------------------------------------------------------------------------- > > Key: ZOOKEEPER-4246 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-4246 > Project: ZooKeeper > Issue Type: Bug > Components: server > Reporter: Martin Kellogg > Priority: Major > Labels: pull-request-available > Time Spent: 10m > Remaining Estimate: 0h > > There are three (related) possible resource leaks in the `getInputStream` > and `getOutputStream` methods in `SnapStream.java`. I noticed the first > because of the use of the error-prone `GZIPOutputStream`, and the other two > after looking at the surrounding code. > Here is the offending code (copied from > [here|https://github.com/apache/zookeeper/blob/master/zookeeper-server/src/main/java/org/apache/zookeeper/server/persistence/SnapStream.java#L102]): > {noformat} > /** > * Return the CheckedInputStream based on the extension of the fileName. > * > * @param file the file the InputStream read from > * @return the specific InputStream > * @throws IOException > */ > public static CheckedInputStream getInputStream(File file) throws > IOException { > FileInputStream fis = new FileInputStream(file); > InputStream is; > switch (getStreamMode(file.getName())) { > case GZIP: > is = new GZIPInputStream(fis); > break; > case SNAPPY: > is = new SnappyInputStream(fis); > break; > case CHECKED: > default: > is = new BufferedInputStream(fis); > } > return new CheckedInputStream(is, new Adler32()); > } > /** > * Return the OutputStream based on predefined stream mode. > * > * @param file the file the OutputStream writes to > * @param fsync sync the file immediately after write > * @return the specific OutputStream > * @throws IOException > */ > public static CheckedOutputStream getOutputStream(File file, boolean > fsync) throws IOException { > OutputStream fos = fsync ? new AtomicFileOutputStream(file) : new > FileOutputStream(file); > OutputStream os; > switch (streamMode) { > case GZIP: > os = new GZIPOutputStream(fos); > break; > case SNAPPY: > os = new SnappyOutputStream(fos); > break; > case CHECKED: > default: > os = new BufferedOutputStream(fos); > } > return new CheckedOutputStream(os, new Adler32()); > } > {noformat} > All three possible resource leaks are caused by the constructors of the > intermediate streams (i.e. `is` and `os`), some of which might throw > `IOException`s: > * in `getOutputStream`, the call to `new GZIPOutputStream` can throw an > exception, because `GZIPOutputStream` writes out the header in the > constructor. If it does throw, then `fos` is never closed. That it does so > makes it hard to use correctly; someone raised this as an issue with the JDK > folks [here|https://bugs.openjdk.java.net/browse/JDK-8180899], but they > closed it as "won't fix" because the constructor is documented to throw > (hence the need to catch the exception here). > * in `getInputStream`, the call to `new GZIPInputStream` can throw an > `IOException` for a similar reason, causing the file handle held by `fis` to > leak. > * similarly, the call to `new SnappyInputStream` can throw an `IOException`, > because it tries to read the file header during construction, which also > causes `fis` to leak. `SnappyOutputStream` cannot throw; I checked > [here|https://github.com/xerial/snappy-java/blob/master/src/main/java/org/xerial/snappy/SnappyOutputStream.java]. > I'll submit a PR with a (simple) fix shortly after this bug report goes up > and gets assigned an issue number, and add a link to this issue. -- This message was sent by Atlassian Jira (v8.3.4#803005)