Punto muerto al llamar a dos métodos sincronizados

Question

class Downloader extends Thread {
    private InputStream in;
    private OutputStream out;
    private ArrayList<ProgressListener> listeners;
    public Downloader(URL url, String outputFilename) throws IOException {
        in = url.openConnection().getInputStream();
        out = new FileOutputStream(outputFilename);
        listeners = new ArrayList<ProgressListener>();
    }
    public synchronized void addListener(ProgressListener listener) {
        listeners.add(listener);
    }
    public synchronized void removeListener(ProgressListener listener) {
        listeners.remove(listener);
    }

    private synchronized void updateProgress(int n) {
        for (ProgressListener listener: listeners)
            listener.onProgress(n);
    }
    public void run() {
        int n = 0, total = 0;
        byte[] buffer = new byte[1024];
        try {
            while((n = in.read(buffer)) != -1) {
                out.write(buffer, 0, n);
                total += n;
                updateProgress(total);
            }
            out.flush();
        } catch (IOException e) { }
    }
}

El código anterior es del libro "siete modelos de concurrencia en siete semanas".El libro dice que el código anterior tiene potencial para provocar un punto muerto, ya que el método sincronizado updateProgress llama a un método extraño [onProgress] que podría adquirir otro bloqueo.Dado que adquirimos dos bloqueos sin el orden correcto, podría producirse un punto muerto.

¿Alguien puede explicar cómo se produce el punto muerto en el escenario anterior?

Gracias de antemano.

Answer 1

Lo mejor es hacer que los objetos que utilizas synchronized privado.

Desde que sincronizas en el Downloader, no sabes si otros hilos se sincronizan en el Downloader también.

El siguiente oyente provoca un punto muerto:

MyProgressListener extends ProgressListener {

     public Downloader downloader;
     public void onProgress(int n) {
         Thread t = new Thread() {
             @Override
             public void run() {
                 synchronized(downloader) {
                     // do something ...
                 }
             }
         };
         t.start();
         t.join();
     }
}

Código que bloquea:

Downloader d = new Downloader(...);
MyProgressListener l = new MyProgressListener();
l.downloader = d;
d.addListener(l);
d.run();

Lo siguiente sucederá si ejecuta ese código:

1. el hilo principal llega al updateProgress y adquiere un candado en el Downloader
2. el MyProgressListener's onProgress Se llama al método y se crea el nuevo hilo. t Está empezado
3. el hilo principal llega t.join();

En esta situación, el hilo principal no puede continuar hasta t está terminado, pero por t Para terminar, el hilo principal tendría que liberar su bloqueo en el Downloader, pero eso no sucederá ya que el hilo principal no puede continuar -> Punto muerto

Answer 2

En primer lugar, recuerda que la palabra clave synchronized, cuando se aplica a una clase, implica bloquear todo el objeto al que pertenece este método.Ahora, esbocemos otro par de objetos que desencadenan el punto muerto:

class DLlistener implements ProgressListener {

  private Downloader d;

  public DLlistener(Downloader d){
      this.d = d;
      // here we innocently register ourself to the downloader: this method is synchronized
      d.addListener(this);
  }

  public void onProgress(int n){
    // this method is invoked from a synchronized call in Downloader
    // all we have to do to create a dead lock is to call another synchronized method of that same object from a different thread *while holding the lock*
    DLthread thread = new DLThread(d);
    thread.start();
    thread.join();
  }
}

// this is the other thread which will produce the deadlock
class DLThread extends Thread {
   Downloader locked;
  DLThread(Downloader d){
    locked = d;
  }
  public void run(){
    // here we create a new listener, which will register itself and generate the dead lock
    DLlistener listener(locked);
    // ...
  }
}

Una forma de evitar el punto muerto es posponer el trabajo realizado en addListener al tener colas internas de oyentes esperando ser agregados/eliminados, y tener Downloader cuidándolos por sí mismo periódicamente.Esto depende en última instancia de Downloader.run funcionamiento interno, por supuesto.

Answer 3

Probablemente el problema en este código:

for (ProgressListener listener: listeners)
            listener.onProgress(n);

Cuando un hilo, que sostiene un bloqueo, llama un método externo Como este (OnProgress), entonces no puedes garantizar que La implementación de este método no intentará obtener otra cerradura, que podría ser sostenido por diferentes hilos.Esto puede causar un punto muerto.

Answer 4

Here's a classic example that shows the kind of hard-to-debug problems the author is trying to avoid.

The class UseDownloader is created and downloadSomething is called.

As the download progresses, the onProgress method is called. Since this is called from within the synchronized block, the Downloader motinor is locked. Inside our onProgress method, we need to lock our own resource, in this case lock. So when we are trying to synchronize on lock we are holding the Downloader monitor.

If another thread has decided that the download should be canceled, it will call setCanceled. This first tests done so it synchronized on the lock monitor and then calls removeListener. But removeListener requires the Downloader lock.

This kind of deadlock can be hard to find because it doesn't happen very often.

  public static final int END_DOWNLOAD = 100;

  class UseDownloader implements ProgressListener {
    Downloader d;
    Object lock = new Object();
    boolean done = false;

    public UseDownloader(Downloader d) {
      this.d = d;
    }
    public void onProgress(int n) {
      synchronized(lock) {
        if (!done) {
          // show some progress
        }
      }
    }

    public void downloadSomething() {
      d.addListener(this);
      d.start();
    }

    public boolean setCanceled() {
      synchronized(lock) {
        if (!done) {
          done = true;
          d.removeListener(this);
        }
      }
    }
  }

Answer 5

The following example leads to a deadlock because the MyProgressListener tries to acquire the Downloader lock while it's already acquired.

class MyProgressListener extends ProgressListener {
    private Downloader myDownloader;

    public MyProgressListener(Downloader downloader) {
        myDownloader = downloader;
    }

    public void onProgress(int n) {
        // starts and waits for a thread that accesses myDownloader
    }
}

Downloader downloader = new Downloader(...);
downloader.addListener(new MyListener(downloader));
downloader.run();