java - 这里哪些同步语句是不必要的？

Question

首先是代码片段...

final class AddedOrders {
    private final Set<Order> orders = Sets.newConcurrentHashSet();
    private final Set<String> ignoredItems = Sets.newConcurrentHashSet(); 
    private boolean added = false;

    public synchronized void clear() {
        added = false;
    }

    public synchronized void add(Order order) {
        added = orders.add(order);
    }

    public synchronized void remove(Order order) {
        if (added) orders.remove(order); 
    }

    public synchronized void ban(String item) {
        ignoredItems.add(item);
    }

    public synchronized boolean has(Order order) {
        return orders.contains(order);
    }

    public synchronized Set<Order> getOrders() {
        return orders;
    }

    public synchronized boolean ignored(String item) {
        return ignoredItems.contains(item);
    }
}

private final AddedOrders added = new AddedOrders();


...
    boolean subscribed;
    int i = 10;
    synchronized (added) {
        while (!(subscribed = client.getSubscribedOrders().containsAll(added.getOrders())) && (i>0)) {
           Helper.out("...order not subscribed yet (try: %d)", i);
           Thread.sleep(200);
           i--;
        }
    }

我想知道什么...

有人能指出哪个 synchronized没有必要吗？

当然这不是完整的代码，但假设在完整的项目中调用了所有方法，并且在先检查值，然后修改中调用了一些方法组合风格

added (类)被多个 Thread 访问

client是外部服务器 API 的一部分，我还不完全确定它是否是线程安全的，但我认为它一定是

ConcurrentHashSet是 google guava Class但它是基于ConcurrentHashMap显然，文档说它具有所有相同的并发保证。

但我并不完全理解这些保证是什么，尽管我做了一些阅读。也就是说，我知道仅仅检查并设置同步的值是不行的 HashMap (无需使用 synchronized Map 在 synchronized block 上同步)，但是我不知道您是否可以在 ConcurrentHashMap 中做到这一点或不(不使用 ConcurrentHashMap 在 synchronized block 上同步)。

Answer 1

代码中真正需要同步的唯一情况是测试或更新添加的标志的情况。您需要同步块(synchronized block)来确保对标志的更改在线程之间可见，并且还需要确保添加的标志更改与订单数据结构的更改同步进行。同步关键字可防止另一个线程介入并在检查标志和更改数据结构之间执行某些操作(如果删除同步，则删除方法可能会像这样被破坏)。

最后的代码似乎有问题，因为您锁定了添加的对象，然后不释放锁定，任何其他线程都没有机会进行该线程正在寻找的更改。虽然看起来你正在等待另一个对象发生变化，所以这个批评可能是无效的。不过，带着锁 sleep 似乎很危险。这种事情就是为什么 Object#wait 释放它获取的锁的原因。

另请注意，由于您将引用传递给 Orders 集，因此此类外部的代码可以添加订单。您应该采取一些措施来保护这些内部数据，例如将其包装在 immutableSet 中返回，以便调用者无法进行更改。

一般来说，当您想要对更改施加一定的粒度时，您需要同时进行 2 个或更多更改，而无需交错，则使用同步。一个例子是检查然后执行序列，其中您执行一些代码，根据其他内容的值进行更改，并且您不希望在检查和操作之间执行其他线程(因此决定可以采取行动，然后允许该行动的条件发生变化，因此该行动可能无效)。如果单个值发生更改但它们不相关，那么您可以将它们设置为 volatile 或使用原子变量，并减少必须执行的锁定量。

在像clear方法这样的情况下，可以删除synchronized关键字，这是一个有效的观点，其中唯一改变的是添加的标志，该标志可以设置为 volatile 的。我仍然不明白添加标志的目的。输入已经存在的值的任何内容都可以将标志返回为 false，如果同时修改此结构，则根据标志的当前值推理任何操作是否有意义，这一点并不明显。

在不知道确切的上下文的情况下，很难说，但一般来说，在没有考虑与多个线程一起使用的情况下创建的类可能需要在并发环境中使用之前进行广泛的修改。