You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@avro.apache.org by dk...@apache.org on 2018/12/20 20:55:25 UTC
[avro] branch master updated: Fix for AVRO-2286 and regression test
(#406)
This is an automated email from the ASF dual-hosted git repository.
dkulp pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/avro.git
The following commit(s) were added to refs/heads/master by this push:
new a262a10 Fix for AVRO-2286 and regression test (#406)
a262a10 is described below
commit a262a107ddd403e3ac5a773c735110a6565104de
Author: Martin Jubelgas <ma...@gmx.de>
AuthorDate: Thu Dec 20 21:55:21 2018 +0100
Fix for AVRO-2286 and regression test (#406)
---
.../java/org/apache/avro/file/DataFileReader.java | 30 +++++++++--
.../java/org/apache/avro/file/DataFileWriter.java | 9 +++-
.../java/org/apache/avro/TestDataFileReader.java | 62 ++++++++++++++++++++++
3 files changed, 95 insertions(+), 6 deletions(-)
diff --git a/lang/java/avro/src/main/java/org/apache/avro/file/DataFileReader.java b/lang/java/avro/src/main/java/org/apache/avro/file/DataFileReader.java
index 0c399a9..5fccf9e 100644
--- a/lang/java/avro/src/main/java/org/apache/avro/file/DataFileReader.java
+++ b/lang/java/avro/src/main/java/org/apache/avro/file/DataFileReader.java
@@ -25,6 +25,7 @@ import java.util.Arrays;
import org.apache.avro.InvalidAvroMagicException;
import org.apache.avro.io.DecoderFactory;
+import org.apache.commons.compress.utils.IOUtils;
import org.apache.avro.io.DatumReader;
import static org.apache.avro.file.DataFileConstants.SYNC_SIZE;
import static org.apache.avro.file.DataFileConstants.MAGIC;
@@ -40,7 +41,13 @@ public class DataFileReader<D>
/** Open a reader for a file. */
public static <D> FileReader<D> openReader(File file, DatumReader<D> reader)
throws IOException {
- return openReader(new SeekableFileInput(file), reader);
+ SeekableFileInput input = new SeekableFileInput( file );
+ try {
+ return openReader( input, reader );
+ } catch ( final Throwable e ) {
+ IOUtils.closeQuietly( input );
+ throw e;
+ }
}
/** Open a reader for a file. */
@@ -87,16 +94,29 @@ public class DataFileReader<D>
/** Construct a reader for a file. */
public DataFileReader(File file, DatumReader<D> reader) throws IOException {
- this(new SeekableFileInput(file), reader);
+ this(new SeekableFileInput(file), reader, true);
}
/** Construct a reader for a file. */
public DataFileReader(SeekableInput sin, DatumReader<D> reader)
throws IOException {
+ this(sin, reader, false);
+ }
+
+ /** Construct a reader for a file. */
+ protected DataFileReader(SeekableInput sin, DatumReader<D> reader, boolean closeOnError)
+ throws IOException {
super(reader);
- this.sin = new SeekableInputStream(sin);
- initialize(this.sin);
- blockFinished();
+ try {
+ this.sin = new SeekableInputStream(sin);
+ initialize(this.sin);
+ blockFinished();
+ } catch(final Throwable e) {
+ if (closeOnError) {
+ IOUtils.closeQuietly( sin );
+ }
+ throw e;
+ }
}
/** Construct using a {@link DataFileStream.Header}. Does not call {@link
diff --git a/lang/java/avro/src/main/java/org/apache/avro/file/DataFileWriter.java b/lang/java/avro/src/main/java/org/apache/avro/file/DataFileWriter.java
index 37c4613..a185172 100644
--- a/lang/java/avro/src/main/java/org/apache/avro/file/DataFileWriter.java
+++ b/lang/java/avro/src/main/java/org/apache/avro/file/DataFileWriter.java
@@ -40,6 +40,7 @@ import org.apache.avro.generic.GenericDatumReader;
import org.apache.avro.io.BinaryEncoder;
import org.apache.avro.io.DatumWriter;
import org.apache.avro.io.EncoderFactory;
+import org.apache.commons.compress.utils.IOUtils;
/** Stores in a file a sequence of data conforming to a schema. The schema is
* stored in the file with the data. Each datum in a file is of the same
@@ -126,7 +127,13 @@ public class DataFileWriter<D> implements Closeable, Flushable {
/** Open a new file for data matching a schema with a random sync. */
public DataFileWriter<D> create(Schema schema, File file) throws IOException {
- return create(schema, new SyncableFileOutputStream(file), null);
+ SyncableFileOutputStream sfos = new SyncableFileOutputStream(file);
+ try {
+ return create(schema, sfos, null);
+ } catch (final Throwable e) {
+ IOUtils.closeQuietly(sfos);
+ throw e;
+ }
}
/** Open a new file for data matching a schema with a random sync. */
diff --git a/lang/java/avro/src/test/java/org/apache/avro/TestDataFileReader.java b/lang/java/avro/src/test/java/org/apache/avro/TestDataFileReader.java
new file mode 100644
index 0000000..0676036
--- /dev/null
+++ b/lang/java/avro/src/test/java/org/apache/avro/TestDataFileReader.java
@@ -0,0 +1,62 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.avro;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.fail;
+import java.io.IOException;
+import java.lang.management.ManagementFactory;
+import java.lang.management.OperatingSystemMXBean;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import org.apache.avro.file.DataFileReader;
+import org.apache.avro.generic.GenericDatumReader;
+import org.junit.Test;
+import com.sun.management.UnixOperatingSystemMXBean;
+
+@SuppressWarnings("restriction")
+public class TestDataFileReader {
+
+ @Test
+ // regression test for bug AVRO-2286
+ public void testForLeakingFileDescriptors() throws IOException {
+ Path emptyFile = Files.createTempFile("empty", ".avro");
+ Files.deleteIfExists(emptyFile);
+ Files.createFile(emptyFile);
+
+ long openFilesBeforeOperation = getNumberOfOpenFileDescriptors();
+ try (DataFileReader<Object> reader = new DataFileReader<>(emptyFile.toFile(), new GenericDatumReader<>())) {
+ fail("Reading on empty file is supposed to fail.");
+ } catch (IOException e) {
+ // everything going as supposed to
+ }
+ Files.delete(emptyFile);
+
+ assertEquals("File descriptor leaked from new DataFileReader()", openFilesBeforeOperation, getNumberOfOpenFileDescriptors());
+ }
+
+ private long getNumberOfOpenFileDescriptors() {
+ OperatingSystemMXBean osMxBean = ManagementFactory.getOperatingSystemMXBean();
+ if (osMxBean instanceof UnixOperatingSystemMXBean) {
+ return ((UnixOperatingSystemMXBean)osMxBean).getOpenFileDescriptorCount();
+ }
+ return 0;
+ }
+
+
+}