Closing Files that Weren't Successfully Opened

[Michal Kleczek]

Hence the 'catch' block. What resource can leak there?

A file handle. You do not call close() on FileReader in case BufferedReader
constructor throws.

 

[Lew]

A file handle. You do not call close() on FileReader in case BufferedReader
constructor throws.

Non-issue. If FileReader doesn't then BufferedReader won't.

 

[Michal Kleczek]

Non-issue. If FileReader doesn't then BufferedReader won't.

1. It can always throw a RuntimeException or an Error. Depending on
situation it should or should not be handled gracefully hence it _can_ be an
issue. If it is a library (reusable) code it certainly is.

2. My original reply to Robert's post was:

<quote>
That only works because both InputStreamReader and BufferedReader
constructors don't throw.
But it is not safe to do the same for:
final ObjectInputStream ois =
new ObjectInputStream(new FileInputStream("foo.txt"));
</quote>

You've read it before replying, haven't you?

 

[Andreas Leitgeb]

Michal Kleczek wrote:
public void useResource( String src, String dst ) {
final BufferedReader reader;
try {
reader = new BufferedReader( new FileReader( src ));
} catch ( IOException exc ) {
[...]
is not exception safe - if BufferedReader constructor throws you've got a
resource leak.
Hence the 'catch' block. What resource can leak there?

A file handle. You do not call close() on FileReader in case BufferedReader
constructor throws.

Non-issue. If FileReader doesn't then BufferedReader won't.

How is a leaked(for uncontrollable time) file handle a non-issue?
The other statement is unclear, but could be taken as confirming
the issue: FileReader will free the filehandle not before it is
garbage-collected and (as you said) BufferedReader won't, either.

 

[Lew]

1. It can always throw a RuntimeException or an Error. Depending on
situation it should or should not be handled gracefully hence it _can_ be an
issue. If it is a library (reusable) code it certainly is.

If it throws an Error your resources are released because the program crashes.
Same for RuntimeException.

2. My original reply to Robert's post was:

<quote>
That only works because both InputStreamReader and BufferedReader
constructors don't throw.
But it is not safe to do the same for:
final ObjectInputStream ois =
new ObjectInputStream(new FileInputStream("foo.txt"));
</quote>

You've read it before replying, haven't you?

and you were mistaken then, too.

 

[Andreas Leitgeb]

Lew wrote:
Michal Kleczek wrote:
public void useResource( String src, String dst ) {
final BufferedReader reader;
try {
reader = new BufferedReader( new FileReader( src ));
} catch ( IOException exc ) {
[...]
is not exception safe - if BufferedReader constructor throws you've got a
resource leak.
Hence the 'catch' block. What resource can leak there?
A file handle. You do not call close() on FileReader in case BufferedReader
constructor throws.

Non-issue. If FileReader doesn't then BufferedReader won't.

How is a leaked(for uncontrollable time) file handle a non-issue?

ok, got it: BufferedReader doesn't throw, making it a non-issue for
these particular classes.

The other statement is unclear, ...

It was unclear, because the actual verb was missing. Obviously now
it was "throw", not (as I previously thought "cleanup").

 

[Michal Kleczek]

If it throws an Error your resources are released because the program
crashes.
Same for RuntimeException.

I know of a lot of cases when the program will not (and should not) crash
after RuntimeException (or even an Error).
Are you suggesting to write non-robust code leading to potential resource
leaks?

and you were mistaken then, too.

Where?

 

[Lew]

I know of a lot of cases when the program will not (and should not) crash
after RuntimeException (or even an Error).
Are you suggesting to write non-robust code leading to potential resource
leaks?
No.

If FileInputStream doesn't have an I/O error, then ObjectInputStream won't,
either. The I/O condition will have been handled by the time the OIS is
constructed.

BOOYAH!

 

[Lew]

It might:

int erronousSize = -2048;
BufferedReader reader = new BufferedReader( new FileReader( "test.txt" ), erronousSize );

What exception does that throw?

Anyway, that wasn't the form of the constructor under discussion, was it?
There was no "size" argument in the example under controversy, so your point
is irrelevant.

Anyway, surely the particular behaviour of BufferedReader is
tangential to the discussion? It's the problems of the general
pattern that's of real interest.

Precisely. If these issues that Michal Kleczek is so worried about are
actually a problem, just extend the pattern. Is that so difficult to infer,
Michal, really? Just because you think an example is incomplete doesn't mean
the point isn't valid, right, Michal?

BOOYAH!

 

[Michal Kleczek]

If FileInputStream doesn't have an I/O error, then ObjectInputStream
won't,
either. The I/O condition will have been handled by the time the OIS is
constructed.

ObjectInputStream constructor can also throw IOException (actually its
subclass: StreamCorruptedException) if the underlying InputStream does not
start with a valid header.

 

[Lew]

ObjectInputStream constructor can also throw IOException (actually its
subclass: StreamCorruptedException) if the underlying InputStream does not
start with a valid header.

OK, good point, you're right - so extend the pattern to cover that, then.

BOOYAH!

 

[Michal Kleczek]

Precisely. If these issues that Michal Kleczek is so worried about are
actually a problem, just extend the pattern. Is that so difficult to
infer,
Michal, really? Just because you think an example is incomplete doesn't
mean the point isn't valid, right, Michal?

Oh, sure.
But it is so much fun to point out (potential) issues in _other_ people's
code

 

[Lew]

Oh, sure.
But it is so much fun to point out (potential) issues in _other_ people's
code

OK, now you're my hero. I should mention that your comments were absolutely
correct. Thanks for your participation.

I am being serious.