Why does BufferedInputStream copy a field to a local variable rather than use the field directly
If you look at this code out of context there is no good explanation for that "alias". It is simply redundant code or poor code style.
But the context is that BufferedInputStream
is a class that can be subclassed, and that it needs to work in a multi-threaded context.
The clue is that in
is declared in FilterInputStream
is protected volatile
. That means that there is a chance that a subclass could reach in and assign null
to in
. Given that possibility, the "alias" is actually there to prevent a race condition.
Consider the code without the "alias"
private InputStream getInIfOpen() throws IOException {
if (in == null)
throw new IOException("Stream closed");
return in;
}
- Thread A calls
getInIfOpen()
- Thread A evaluates
in == null
and sees thatin
is notnull
. - Thread B assigns
null
toin
. - Thread A executes
return in
. Which returnsnull
becausea
is avolatile
.
The "alias" prevents this. Now in
is read just once by thread A. If thread B assigns null
after thread A has in
it doesn't matter. Thread A will either throw an exception or return a (guaranteed) non-null value.
This is because the class BufferedInputStream
is designed for multi-threaded usage.
Here, you see the declaration of in
, which is placed in the parent class FilterInputStream
:
protected volatile InputStream in;
Since it is protected
, its value can be changed by any subclass of FilterInputStream
, including BufferedInputStream
and its subclasses. Also, it is declared volatile
, which means that if any thread changes the value of the variable, this change will immediately be reflected in all other threads. This combination is bad, since it means the class BufferedInputStream
has no way to control or know when in
is changed. Thus, the value can even be changed between the check for null and the return statement in BufferedInputStream::getInIfOpen
, which effectively makes the check for null useless. By reading the value of in
only once to cache it in the local variable input
, the method BufferedInputStream::getInIfOpen
is safe against changes from other threads, since local variables are always owned by a single thread.
There is an example in BufferedInputStream::close
, which sets in
to null:
public void close() throws IOException {
byte[] buffer;
while ( (buffer = buf) != null) {
if (bufUpdater.compareAndSet(this, buffer, null)) {
InputStream input = in;
in = null;
if (input != null)
input.close();
return;
}
// Else retry in case a new buf was CASed in fill()
}
}
If BufferedInputStream::close
is called by another thread while BufferedInputStream::getInIfOpen
is executed, this would result in the race condition described above.
This is such a short code, but, theoretically, in a multi-threaded environment, in
may change right after the comparison, so the method could return something it didn't check (it could return null
, thus doing the exact thing it was meant to prevent).