2008-12-01 23 views
5

Checkstyle informa este código como "El idioma de bloqueo comprobado doble está roto", pero no creo que mi código realmente se vea afectado por los problemas con el bloqueo comprobado.¿Este bloqueo doble está roto?

Se supone que el código crea una fila en una base de datos si una fila con esa identificación no existe. Se ejecuta en un entorno de subprocesos múltiples y quiero evitar las excepciones SQL de clave primaria.

El pseudo-código:

private void createRow(int id) { 
    Row row = dao().fetch(id); 
    if (row == null) { 
    synchronized (TestClass.class) { 
     row = dao().fetch(id); 
     if (row == null) { 
      dao().create(id); 
     } 
    } 
    } 
} 

estoy de acuerdo que se parezca bloqueo doble comprobación, pero no estoy usando variables estáticas y el código de fetch() y crear() es probablemente demasiado compleja para ser inlined y poner fuera de servicio.

¿Estoy equivocado o marcado? :)

Respuesta

4

Suponiendo que desea que la línea más interna a leer:

row = dao().create(id); 

No es un problema de bloqueo de doble comprobación clásico asumiendo dao().fetch se mutexed correctamente del método de crear.

Editar: (código se actualiza)

El problema clásico de una cerradura de doble comprobación está teniendo un valor asignado antes de la inicialización se produce cuando dos hilos están accediendo al mismo valor.

Suponiendo que el DAO está sincronizado correctamente y no devolverá un valor parcialmente inicializado, esto no adolece de los defectos del idioma de bloqueo comprobado.

+1

La suposición de que DAO está sincronizado correctamente es CRÍTICA. –

+0

El problema con esta suposición es que el DAO no tiene idea de lo que espera de él y la próxima posibilidad de código podría arruinar sus esperanzas ... Por lo tanto, creo que esta respuesta es incorrecta * y * ¡peligrosa! –

+0

¿Quiere decir que debería esperarse que el DAO devuelva instancias de objetos que no se inicializan? ¿Por qué importaría si hubiera un bloqueo si puede devolver objetos en mal estado de todos modos? – Dustin

5

Creo que en este caso, el estilo de comprobación es el correcto. En su código como se presenta, considere lo que sucedería si dos subprocesos tuvieran row == null en la entrada al bloque sincronizado. El hilo A entraría en el bloque e insertaría la nueva fila. Luego, después de que el hilo A sale del bloque, el hilo B ingresará en el bloque (porque no sabe lo que acaba de suceder) e intenta insertar la misma nueva fila nuevamente.

Veo que acaba de cambiar el código y agregó una línea que falta bastante importante allí. En el nuevo código, es posible que pueda salirse con la suya, ya que dos subprocesos no dependerán de los cambios en una variable compartida (estática). Pero es mejor que veas si tu DBMS admite una declaración como INSERT OR UPDATE.

Otra buena razón para delegar esta funcionalidad en el DBMS es si alguna vez necesita implementar más de un servidor de aplicaciones. Como los bloques synchronized no funcionan en todas las máquinas, de todos modos tendrá que hacer algo más en ese caso.

+0

Sí, el código se está ejecutando en un contenedor EJB, así que sé que no puedo usar la sincronización, ya que fallará cuando se agrupe. Comprobaré si DB2 tiene alguna función de "inserción o actualización" que pueda usar. –

+0

Entonces, ¿es el peor de los casos que la sincronización falla y hay dos intentos en una inserción (y uno falla)? Quizás sea mejor simplemente no molestarse con la sincronización y simplemente detectar el error de inserción cuando ocurre la carrera. – Dustin

+0

Sí, esa sería una solución simple. El problema es que nuestro marco interno detecta y registra la excepción SQLException antes de que mi aplicación tenga la oportunidad de hacerlo.Por lo tanto, no puedo evitar que la excepción se registre y el software de vigilancia supervisa los registros. –

3

Si usted está tentado a escribir código como este, tenga en cuenta:

  • Desde Java 1.4, métodos de sincronización se ha convertido en bastante barato. No es gratis, pero el tiempo de ejecución realmente no sufre tanto que valga la pena arriesgarse a la corrupción de datos.

  • Desde Java 1.5, tiene las clases Atomic * que le permiten leer y establecer campos de forma atómica. Lamentablemente, no resuelven su problema. Por qué no agregaron AtomicCachedReference o algo así (que llamaría a un método invalidable cuando se llama a get() y el valor actual == null) me supera.

  • Probar ehcache. Le permite configurar un caché (es decir, un objeto que le permite llamar al código si una clave es no incluida en un mapa). Esto es generalmente lo que quiere y los cachés realmente resuelven su problema (y todos esos otros problemas que usted no sabía que existían).

2

Como otros han señalado, este código va a hacer lo que se propone como está, pero sólo bajo un estricto conjunto de suposiciones no son obvios:

  1. El código Java es no agrupado (ver Respuesta de @Greg H)
  2. La referencia de "fila" es que solo se ha verificado nulo en la primera línea, antes del bloque de sincronización.

La razón el idioma una doble comprobación de bloqueo se rompe (por sección 16.2.4 de Java Concurrency in Practice) es que es posible que un hilo de ejecutar este método para ver un no nulo pero inicializa incorrectamente referencia a "fila ", antes de ingresar al bloque sincronizado (a menos que" dao "proporcione la sincronización adecuada). Si su método estaba haciendo algo con "fila", aparte de comprobar que es nulo o no, se rompería. Tal como está, probablemente esté bien, pero muy frágil - personalmente No me sentiría cómodo cometiendo este código si pensara que había alguna posibilidad remota de que algún otro desarrollador en algún momento posterior modifique el método sin entender las sutilezas de DCL.

Cuestiones relacionadas