[ https://issues.apache.org/jira/browse/ZOOKEEPER-4243?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Damien Diederen resolved ZOOKEEPER-4243. ---------------------------------------- Resolution: Duplicate > Resource leaks in > org.apache.zookeeper.server.persistence.SnapStream#getInputStream and > #getOutputStream > -------------------------------------------------------------------------------------------------------- > > Key: ZOOKEEPER-4243 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-4243 > Project: ZooKeeper > Issue Type: Bug > Components: server > Reporter: Martin Kellogg > Priority: Major > > 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 > [https://github.com/apache/zookeeper/blob/master/zookeeper-server/src/main/java/org/apache/zookeeper/server/persistence/SnapStream.java#L102):] > > {code:java} > /** > * 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()); > }{code} > All three possible resource leaks are caused by the constructors of the > intermediate streams (i.e. is and os), some of which might throw IOExceptions: > * 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 why we > 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 will submit a PR with a fix on Github shortly and update this description > with a link. -- This message was sent by Atlassian Jira (v8.3.4#803005)