-
Enhancement
-
Resolution: Won't Fix
-
P4
-
None
-
6
-
x86
-
windows_xp
A DESCRIPTION OF THE REQUEST :
ObjectInputStream is designed to be an adapter over arbitrary input stream. It takes ownership on this stream in sense that when ObjectInputStream is closed, the underlying input stream is also closed. So this pattern of using ObjectInputStream is expected to be legal.
ObjectInputStream oos = new ObjectInputStream(new FileInputStream(file));
try {
oos.readObject();
} finally {
oos.close();
}
But unfortunately it is not. If constructor of ObjectInputStream throws an exception (e.g. when file is empty) the underlying FileInputStream remains open. It would be good if constructor would close underlying stream in exceptional cases, or there were some exception-safe factory method for this purpose.
PS many other implementation of readers, writers and streams in java.io.* package have the same problem. They have to be revised as well.
JUSTIFICATION :
This is necessary in order to avoid resource leaks and to keep code clear.
EXPECTED VERSUS ACTUAL BEHAVIOR :
EXPECTED -
attached junit passes
ACTUAL -
attached junit fails
---------- BEGIN SOURCE ----------
// Note: Windows prevents deleting files that are remains open.
import java.io.EOFException;
import java.io.File;
import java.io.FileInputStream;
import java.io.IOException;
import java.io.InputStream;
import java.io.ObjectInputStream;
import org.junit.After;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
public class ObjectInputStreamBug {
private File file;
private InputStream toclose;
@Before
public void setUp() throws IOException {
file = File.createTempFile("ObjectInputStreamBug", null);
toclose = null;
}
@After
public void tearDown() throws IOException {
try {
if (toclose != null) {
// new FileInputStream() has been succeeded but hasn't been closed,
// so here is the last chance to close it.
toclose.close();
}
} finally {
if (file.exists()) {
file.delete();
}
}
}
@Test
public void testReadFromEmptyFile() throws IOException {
try {
new ObjectInputStream(toclose = new FileInputStream(file));
Assert.fail("new ObjectInputStream() should throw EOFException");
} catch (EOFException exc) {
// ok, expected
}
Assert.assertTrue(file.delete());
toclose = null; // no need to close it twice
}
}
---------- END SOURCE ----------
CUSTOMER SUBMITTED WORKAROUND :
1) store reference to underlying stream in order to have opportunity to close it
InputStream toclose = new FileInputStream(file);
try {
ObjectInputStream oos = new ObjectInputStream(toclose);
toclose = oos;
oos.readObject();
} finally {
toclose.close();
}
2) write own exception-safe factory method
static ObjectOutputStream createObjectOutputStreamSafely(InputStream is) throws IOException {
boolean ok = false;
try {
ObjectOutputStream result = new ObjectOutputStream(is);
ok = true;
return result;
} finally {
if (!ok) {
is.close();
}
}
}
// ....
ObjectInputStream oos = createObjectOutputStreamSafely(new FileInputStream(file));
try {
oos.readObject();
} finally {
oos.close();
}
ObjectInputStream is designed to be an adapter over arbitrary input stream. It takes ownership on this stream in sense that when ObjectInputStream is closed, the underlying input stream is also closed. So this pattern of using ObjectInputStream is expected to be legal.
ObjectInputStream oos = new ObjectInputStream(new FileInputStream(file));
try {
oos.readObject();
} finally {
oos.close();
}
But unfortunately it is not. If constructor of ObjectInputStream throws an exception (e.g. when file is empty) the underlying FileInputStream remains open. It would be good if constructor would close underlying stream in exceptional cases, or there were some exception-safe factory method for this purpose.
PS many other implementation of readers, writers and streams in java.io.* package have the same problem. They have to be revised as well.
JUSTIFICATION :
This is necessary in order to avoid resource leaks and to keep code clear.
EXPECTED VERSUS ACTUAL BEHAVIOR :
EXPECTED -
attached junit passes
ACTUAL -
attached junit fails
---------- BEGIN SOURCE ----------
// Note: Windows prevents deleting files that are remains open.
import java.io.EOFException;
import java.io.File;
import java.io.FileInputStream;
import java.io.IOException;
import java.io.InputStream;
import java.io.ObjectInputStream;
import org.junit.After;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
public class ObjectInputStreamBug {
private File file;
private InputStream toclose;
@Before
public void setUp() throws IOException {
file = File.createTempFile("ObjectInputStreamBug", null);
toclose = null;
}
@After
public void tearDown() throws IOException {
try {
if (toclose != null) {
// new FileInputStream() has been succeeded but hasn't been closed,
// so here is the last chance to close it.
toclose.close();
}
} finally {
if (file.exists()) {
file.delete();
}
}
}
@Test
public void testReadFromEmptyFile() throws IOException {
try {
new ObjectInputStream(toclose = new FileInputStream(file));
Assert.fail("new ObjectInputStream() should throw EOFException");
} catch (EOFException exc) {
// ok, expected
}
Assert.assertTrue(file.delete());
toclose = null; // no need to close it twice
}
}
---------- END SOURCE ----------
CUSTOMER SUBMITTED WORKAROUND :
1) store reference to underlying stream in order to have opportunity to close it
InputStream toclose = new FileInputStream(file);
try {
ObjectInputStream oos = new ObjectInputStream(toclose);
toclose = oos;
oos.readObject();
} finally {
toclose.close();
}
2) write own exception-safe factory method
static ObjectOutputStream createObjectOutputStreamSafely(InputStream is) throws IOException {
boolean ok = false;
try {
ObjectOutputStream result = new ObjectOutputStream(is);
ok = true;
return result;
} finally {
if (!ok) {
is.close();
}
}
}
// ....
ObjectInputStream oos = createObjectOutputStreamSafely(new FileInputStream(file));
try {
oos.readObject();
} finally {
oos.close();
}