1

我不习惯使用同步。下面的代码片段看起来对吗？ 

public void setNewSessionListener(NewSessionListener newSessionListener) {
    if (this.newSessionListener != null)
        synchronized (this.newSessionListener) {
            this.newSessionListener = newSessionListener;
        }
    else
        this.newSessionListener = newSessionListener;
}

更具体地说，我需要执行空检查吗？我有一种直觉，该代码存在根本性的问题。

4

4 回答 4

3

有两个错误。第一个是，如果您访问一个需要同步的字段，您总是必须使用相同的锁来访问它。此外，您必须检查该字段是否为空并写入同一同步块中的该字段，否则当您向该字段写入内容时，它可能已经不为空。

第二个是最好在不改变的东西上同步，换句话说，在静态最终字段或实例本身上。例如，您可以专门为此目的创建一个锁定对象：

private static final Object LOCK = new Object();

然后你会写：

synchronized (LOCK) {
    if (this.newSessionListener == null) this.newSessionListener = newSessionListener;
}
于 2012-06-24T22:31:29.483 回答
2

你的感觉是对的。synchronized您应该在块内进行空值检查。否则该块不会阻止双重初始化。此外，您不应该同步this.newSessionListener您将要更改的对象 - 选择一个将在整个块范围内保留的对象（引用）。这是保证在任何时间点只有一个线程可以进入这个代码块的唯一方法。实现此目的的典型方法是synchronizeon this。或者，您可以在一个private final对象上进行同步，该对象仅用于此目的。

此外，最终您在分支if和else分支中执行相同的任务，这可能不是您想要的。

于 2012-06-24T22:31:06.107 回答
1

这至少是一个非常糟糕的主意。您正在同步一个然后分配给的对象。

因为您使用的是同步的，所以我假设这是异步调用的，它可以由一个线程调用，而另一个线程在此代码中。如果是这样，您并没有锁定一个公共对象，而是锁定了它在该时间点持有的值。

可能，我可能强调，你可以同步（这个）。这将确保对该特定对象的此方法的所有调用都是同步的。并且对该类的其他实例的调用针对该其他对象被锁定 - 但不是跨实例。

如果要跨所有实例化对象进行同步，请调用 synchronized (YourClass)

于 2012-06-24T22:35:17.270 回答
1

这是另一种可能性（我倾向于使用显式锁而不是同步块）：

private ReentrantLock lock = new ReentrantLock();

lock.lock();
try {
  // do your synchronized code here. 
}
finally {
  lock.unlock();
}

虽然只是通过查看您的代码，但我不确定为什么会有一个 if 块。为什么你在一种情况下同步，而不是另一种情况？特别是考虑到您在这两种情况下都在做同样的任务？

于 2012-06-25T10:43:05.147 回答