¿Por qué BufferedInputStream copia un campo en una variable local en lugar de usar el campo directamente?

107

Cuando leo el código fuente de java.io.BufferedInputStream.getInIfOpen(), estoy confundido acerca de por qué escribió un código como este:

/**
 * 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;
}

¿Por qué usa el alias en lugar de usar la variable de campo indirectamente como se muestra a continuación?

/**
 * 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;
}

¿Alguien puede dar una explicación razonable?

Santo
fuente
En Eclipse, no puede pausar un depurador en una ifdeclaración. Podría ser una razón para esa variable de alias. Solo quería tirar eso por ahí. Especulo, por supuesto.
Debosmit Ray el
@DebosmitRay: ¿Realmente no puedes hacer una pausa en la ifdeclaración?
rkosegi
@rkosegi En mi versión de Eclipse, el problema es similar a este . Puede que no sea algo muy común. Y de todos modos, no lo dije en serio (claramente una broma de mal gusto). :)
Debosmit Ray

Respuestas:

119

Si observa este código fuera de contexto, no hay una buena explicación para ese "alias". Es simplemente código redundante o estilo de código deficiente.

Pero el contexto es que BufferedInputStreames una clase que puede subclasificarse y que necesita funcionar en un contexto de subprocesos múltiples.

La pista es que inse declara en FilterInputStreamis protected volatile. Eso significa que hay una posibilidad de que una subclase puede meter la mano y asignar nulla in. Dada esa posibilidad, el "alias" está realmente ahí para evitar una condición de carrera.

Considere el código sin el "alias"

private InputStream getInIfOpen() throws IOException {
    if (in == null)
        throw new IOException("Stream closed");
    return in;
}
  1. Llamadas del hilo A getInIfOpen()
  2. El hilo A evalúa in == nully ve que inno lo es null.
  3. El hilo B se asigna nulla in.
  4. Enhebrar realiza un return in. Que devuelve nullporque aes un volatile.

El "alias" evita esto. Ahora inse lee solo una vez por el hilo A. Si el hilo B se asigna nulldespués del hilo A in, no importa. El hilo A lanzará una excepción o devolverá un valor no nulo (garantizado).

Stephen C
fuente
11
Lo que muestra por qué las protectedvariables son malas en un contexto de subprocesos múltiples.
Mick Mnemonic
2
De hecho lo hace. Sin embargo, AFAIK, estas clases se remontan a Java 1.0. Este es solo otro ejemplo de una mala decisión de diseño que no pudo solucionarse por temor a romper el código del cliente.
Stephen C
2
@StephenC Gracias por la explicación detallada +1. Entonces, ¿eso significa que no deberíamos usar protectedvariables en nuestro código si es multiproceso?
Madhusudana Reddy Sunnapu
3
@MadhusudanaReddySunnapu La lección general es que en un entorno donde varios subprocesos pueden acceder al mismo estado, debe controlar ese acceso de alguna manera. Esa puede ser una variable privada a la que solo se puede acceder a través de setter, podría ser una protección local como esta, podría ser haciendo que la variable escriba una vez de una manera segura para subprocesos.
Chris Hayes
3
@sam - 1) No es necesario que explique todas las condiciones y estados de la carrera. El objetivo de la respuesta es señalar por qué este código aparentemente inexplicable es de hecho necesario. 2) ¿Cómo es eso?
Stephen C
20

Esto se debe a que la clase BufferedInputStreamestá diseñada para uso de subprocesos múltiples.

Aquí, ve la declaración de in, que se coloca en la clase principal FilterInputStream:

protected volatile InputStream in;

Dado que lo es protected, su valor puede ser cambiado por cualquier subclase de FilterInputStream, incluyendo BufferedInputStreamy sus subclases. Además, se declara volatile, lo que significa que si algún hilo cambia el valor de la variable, este cambio se reflejará inmediatamente en todos los demás hilos. Esta combinación es mala, ya que significa que la clase BufferedInputStreamno tiene forma de controlar o saber cuándo inse cambia. Por lo tanto, el valor incluso se puede cambiar entre la verificación de nulo y la declaración de retorno en BufferedInputStream::getInIfOpen, lo que efectivamente hace que la verificación de nulo sea inútil. Al leer el valor de insolo una vez para almacenarlo en caché en la variable local input, el método BufferedInputStream::getInIfOpenes seguro contra cambios de otros subprocesos, ya que las variables locales siempre son propiedad de un solo subproceso.

Hay un ejemplo en BufferedInputStream::close, que se establece inen nulo:

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()
    }
}

Si BufferedInputStream::closees llamado por otro hilo mientras BufferedInputStream::getInIfOpense ejecuta, esto daría como resultado la condición de carrera descrita anteriormente.

Stefan Dollase
fuente
Estoy de acuerdo, en la medida en que estamos viendo cosas como compareAndSet(), CAS, etc., en el código y en los comentarios. También busqué el BufferedInputStreamcódigo y encontré numerosos synchronizedmétodos. Por lo tanto, está diseñado para un uso de subprocesos múltiples, aunque seguro que nunca lo he usado de esa manera. De todos modos, ¡creo que tu respuesta es correcta!
sparc_spread
Esto probablemente tenga sentido porque getInIfOpen()solo se llama desde public synchronizedmétodos de BufferedInputStream.
Mick Mnemonic
6

Este es un código tan corto, pero, teóricamente, en un entorno de subprocesos múltiples, inpuede cambiar justo después de la comparación, por lo que el método podría devolver algo que no verificó (podría regresar null, haciendo así exactamente lo que estaba destinado a hacer evitar).

acdcjunior
fuente
¿Estoy en lo cierto si digo que la referencia in podría cambiar entre el momento en que llama al método y la devolución del valor (en un entorno de subprocesos múltiples)?
Debosmit Ray el
Sí, puedes decir eso. En última instancia, la probabilidad realmente dependerá del caso concreto (todo lo que sabemos con certeza es que inpuede cambiar en cualquier momento).
acdcjunior
4

Creo que capturar la variable de clase inen la variable local inputes para evitar un comportamiento inconsistente si inotro subproceso lo cambia mientras se getInIfOpen()está ejecutando.

Tenga en cuenta que el propietario de ines la clase principal y no la marca como final.

Este patrón se replica en otras partes de la clase y parece ser una codificación defensiva razonable.

Sam
fuente