Deadlock lors de l'appel de deux méthodes synchronisées

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

Le code ci-dessus est tiré du livre "sept modèles de concurrence en sept semaines".Le livre indique que le code ci-dessus présente un potentiel de blocage car la méthode synchronisée updateProgress appelle une méthode extraterrestre [onProgress] qui pourrait acquérir un autre verrou.Puisque nous acquérons deux verrous sans ordre correct, un blocage peut se produire.

Quelqu'un peut-il expliquer comment l'impasse se produit dans le scénario ci-dessus ?

Merci d'avance.

Answer 1

Il est préférable de fabriquer les objets avec lesquels vous utilisez synchronized privé.

Puisque vous synchronisez sur le Downloader, vous ne savez pas si d'autres threads se synchronisent sur le Downloader aussi.

L'écouteur suivant provoque un blocage :

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

Code qui bloque :

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

Ce qui suit se produira si vous exécutez ce code :

1. le fil principal atteint le updateProgress et acquiert un verrou sur le Downloader
2. le MyProgressListenerc'est onProgress la méthode est appelée et le nouveau thread t a démarré
3. le fil conducteur atteint t.join();

Dans cette situation, le thread principal ne peut pas continuer tant que t est fini, mais pour t pour finir, le thread principal devrait libérer son verrou sur le Downloader, mais cela n'arrivera pas puisque le thread principal ne peut pas continuer -> Impasse

Answer 2

Tout d’abord, rappelons que le mot-clé synchronized, lorsqu'il est appliqué à une classe a, implique de verrouiller l'intégralité de l'objet auquel appartient cette méthode.Maintenant, esquissons quelques autres objets déclenchant le blocage :

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

Une façon d'éviter l'impasse est de reporter le travail effectué dans addListener en ayant des files d'attente internes d'écouteurs en attente d'être ajoutés/supprimés, et avoir Downloader en prenant soin de ceux-ci par lui-même périodiquement.Cela dépend finalement de Downloader.run le travail intérieur bien sûr.

Answer 3

Probablement le problème dans ce code:

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

Lorsqu'un thread, qui contient un verrou, appelle une méthode externe comme celui-ci (ONPROGRESS), vous ne pouvez pas garantir que La mise en œuvre de cette méthode n'essaiera pas d'obtenir une autre serrure, qui pourrait être détenu par un fil différent.Cela peut provoquer une impasse.

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