为什么 BufferedInputStream 将字段复制到本地变量，而不是直接使用该字段

当我阅读 java.io.BufferedInputStream.getInIfOpen()的源代码时，我对它为什么写这样的代码感到困惑:

/**
* Check to make sure that underlying input stream has not been
* nulled out due to close; if not return it;
*/
private InputStream getInIfOpen() throws IOException {
InputStream input = in;
if (input == null)
throw new IOException("Stream closed");
return input;
}

为什么它使用别名而不直接使用字段变量 in，如下所示:

/**
* Check to make sure that underlying input stream has not been
* nulled out due to close; if not return it;
*/
private InputStream getInIfOpen() throws IOException {
if (in == null)
throw new IOException("Stream closed");
return in;
}

有人能给个合理的解释吗？

2517 次浏览
小开
最佳答案

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;
}
  1. Thread A calls getInIfOpen()
  2. Thread A evaluates in == null and sees that in is not null.
  3. Thread B assigns null to in.
  4. Thread A executes return in. Which returns null because a is a volatile.

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).

小开

I believe capturing the class variable in to the local variable input is to prevent inconsistent behavior if in is change by another thread while getInIfOpen() is running.

Notice that the owner of in is the parent class and does not mark it as final.

This pattern is replicated in other parts of the class and seems to be reasonable defensive coding.


提交答案