java - 是否需要嵌套同步块(synchronized block)？

Question

我正在编写一个多线程程序并且我有一个具有嵌套同步块(synchronized block)的方法，我想知道我是否需要内部同步或者是否只需要外部同步就足够了。

public class Tester {

    private BlockingQueue<Ticket> q = new LinkedBlockingQueue<>();
    private ArrayList<Long> list = new ArrayList<>();

    public void acceptTicket(Ticket p) {
        try {
            synchronized (q) {
                q.put(p);

                synchronized (list) {
                    if (list.size() < 5) {
                        list.add(p.getSize());
                    } else {
                        list.remove(0);
                        list.add(p.getSize());
                        }
                }
            }
        } catch (InterruptedException ex) {
            Logger.getLogger(Consumer.class.getName()).log(Level.SEVERE, null, ex);
        }

    }
}

编辑:这不是一个完整的类(class)，因为我仍在努力。但本质上我是在尝试模拟售票机。售票机在 BlockingQueue q 中维护一张票列表。每当客户向机器添加一张票时，机器也会跟踪最后 5 张票的价格(ArrayList 列表)。所以我认为我不需要内部同步，因为只有这个类(票证系统)才能访问 list

Answer 1

危险在于:

    synchronized (q) {
        synchronized (list) {
         }
    }

是否有一天你会添加一个方法，或者以这样的顺序调用方法，实际上它也这样做:

    synchronized (list) {
        synchronized (q) {
         }
    }

那么这只是一个死锁定时炸弹。

假设这是一个完整的类，您可以只锁定一个，或者像通常那样，使用方法上的 synchronized 锁定对象本身。对这些私有(private)的所有其他访问也需要同步。

public class Tester {

private BlockingQueue<Ticket> q = new LinkedBlockingQueue<>();
private ArrayList<Long> list = new ArrayList<>();

public synchronized void acceptTicket(Ticket p) {
    try {
      q.put(p);

      if (list.size() < 5) {
        list.add(p.getSize());
      } else {
        list.remove(0);
        list.add(p.getSize());
      }
    } catch (InterruptedException ex) {
        Logger.getLogger(Consumer.class.getName()).log(Level.SEVERE, null, ex);
    }    
}

或清洁工:

public class Tester {

private BlockingQueue<Ticket> q = new LinkedBlockingQueue<>();
private ArrayList<Long> list = new ArrayList<>();

public void acceptTicket(Ticket p) {
    try {
      //this is cleaner, because I don't know what logger class is doing,
      //I want to eliminate chance of deadlock and reduce time we are in lock
      synchronized (this){
        q.put(p);

        if (list.size() < 5) {
          list.add(p.getSize());
        } else {
          list.remove(0);
          list.add(p.getSize());
        }
     }
    } catch (InterruptedException ex) {
        Logger.getLogger(Consumer.class.getName()).log(Level.SEVERE, null, ex);
    }    
}

仍然更干净，但在您的情况下不太可能成为必需品(根据我所见):

public class Tester {

private final Object lockObj = new Object(); //specific object for locking
                                //could use any other private, non-exposed final but
                                //this makes it absolutely clear what I should be
                                //using for locks
private BlockingQueue<Ticket> q = new LinkedBlockingQueue<>();
private ArrayList<Long> list = new ArrayList<>();

public void acceptTicket(Ticket p) {
    try {
      //"this" can be locked by external code outside my control here,
      //so I use a specific private final object lockObj to eliminate deadlocks and
      //provide finer grained locking - reducing contension
      synchronized (lockObj){
        q.put(p);

        if (list.size() < 5) {
          list.add(p.getSize());
        } else {
          list.remove(0);
          list.add(p.getSize());
        }
     }
    } catch (InterruptedException ex) {
        Logger.getLogger(Consumer.class.getName()).log(Level.SEVERE, null, ex);
    }    
}