Deadlock quando si chiamano due metodi sincronizzati

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

.

Il codice sopra è dal libro "Sette modelli di concorrenza in sette settimane".Il libro dice che il codice sopra riportato sta avendo un potenziale per il deadlock come il metodo sincronizzato UpdateProgress chiama un metodo alieno [onpogress] che potrebbe acquisire un altro blocco. Dal momento che acquisiamo due serrature senza il giusto ordine, potrebbe verificarsi il deadlock.

Qualcuno può spiegare come avviene il deadlock nello scenario sopra?

Grazie in anticipo.

Answer 1

È meglio creare gli oggetti che usi con synchronized privato.

Dato che si sincronizza sul Downloader, non si sa se anche altre thread si sincronizzano su Downloader.

Il seguente listener provoca un deadlock:

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

.

Codice che deadlocks:

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

.

Il seguente succederà se si esegue quel codice:

.
1. Il filo principale raggiunge il updateProgress e AQUIRES un blocco sul Downloader
2. Il metodo MyProgressListenertaGCode onProgress è chiamato e viene avviato il nuovo thread t
3. Il thread principale raggiunge t.join();

In questa situazione il thread principale non può procedere fino a quando t è terminato, ma per t per terminare, il thread principale dovrebbe rilasciare il suo blocco sul Downloader, ma ciò non accadrà poiché il thread principale non può procedere> Deadlock

Answer 2

Prima di tutto, richiamare che la parola chiave synchronized, se applicata a una classe A, implica il blocco dell'intero oggetto a cui appartiene questo metodo.Ora, abbozziamo un altro paio di oggetti innescando il deadlock:

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);
    // ...
  }
}

.

Un modo per evitare che il blocco morto è quello di posticipare il lavoro svolto in addListener avendo code interne di ascoltatori in attesa di essere aggiunti / rimossi, e avere Downloader prendendo cura di coloro per sé periodicamente.Questo in ultima analisi dipende dal funzionamento interiore Downloader.run ovviamente.

Answer 3

Probabilmente il problema in questo codice:

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

.

Quando un filo, che contiene un blocco, chiama un metodo esterno come questo (onprogress), allora non puoi garantirlo L'implementazione di questo metodo non cercherà di ottenere un altro blocco, che potrebbe essere tenuto da un filo diverso.Questo può causare un deadlock.

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