Deadlock when calling two synchronized method

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

The above code is from the book "seven concurrency models in seven weeks". The book says the above code is having potential for the deadlock as the the synchronized method updateProgress calls a alien method[onProgress] that might acquire another lock. Since we acquire two locks without right order, the deadlock might occur.

Can anyone explain how the deadlock happens in the above scenario?

Thanks in advance.

Answer 1

It's best to make the objects you use with synchronized private.

Since you synchronize on the Downloader, you don't know whether other threads synchronize on the Downloader too.

The following listener causes a 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();
     }
}

Code that deadlocks:

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

The following will happen if you run that code:

1. the main thread reaches the updateProgress and aquires a lock on the Downloader
2. the MyProgressListener's onProgress method is called and the new thread t is started
3. the main thread reaches t.join();

In this situation the main thread cannot procede until t is finished, but for t to finish, the main thread would have to release it's lock on the Downloader, but that won't happen since the main thread can't procede -> Deadlock