这是双重检查锁定损坏了吗?
Checkstyle 将此代码报告为“双重检查锁定习惯用法已损坏”,但我不认为我的代码实际上受到双重检查锁定问题的影响。
如果具有该 id 的行不存在,则该代码应该在数据库中创建一行。 它在多线程环境中运行,我想避免主键存在 SQL 异常。
伪代码:
private void createRow(int id) {
Row row = dao().fetch(id);
if (row == null) {
synchronized (TestClass.class) {
row = dao().fetch(id);
if (row == null) {
dao().create(id);
}
}
}
}
我同意它看起来像双重检查锁定,但我没有使用静态变量,并且 fetch() 和 create() 中的代码可能太复杂,无法内联和乱序。
是我错了还是checkstyle错了? :)
Checkstyle reports this code as "The double-checked locking idiom is broken", but I don't think that my code actually is affected by the problems with double-checked locking.
The code is supposed to create a row in a database if a row with that id doesn't exist. It runs in a multi-threaded environment and I want to avoid the primary-key-exists SQL-exceptions.
The pseudo-code:
private void createRow(int id) {
Row row = dao().fetch(id);
if (row == null) {
synchronized (TestClass.class) {
row = dao().fetch(id);
if (row == null) {
dao().create(id);
}
}
}
}
I can agree that it looks like double-checked locking, but I am not using static variables and the code in fetch() and create() is probably too complex to be inlined and put out of order.
Am I wrong or checkstyle? :)
如果你对这篇内容有疑问,欢迎到本站社区发帖提问 参与讨论,获取更多帮助,或者扫码二维码加入 Web 技术交流群。
绑定邮箱获取回复消息
由于您还没有绑定你的真实邮箱,如果其他用户或者作者回复了您的评论,将不能在第一时间通知您!
发布评论
评论(4)
我认为在这种情况下,checkstyle 是正确的。 在所提供的代码中,考虑如果两个线程在同步块的入口处都有
row == null
会发生什么。 线程 A 将进入该块,并插入新行。 然后,在线程 A 退出该块后,线程 B 将进入该块(因为它不知道刚刚发生了什么),并尝试再次插入相同的新行。我看到您刚刚更改了代码并在其中添加了一个非常重要的缺失行。 在新代码中,您可能能够摆脱这种情况,因为两个线程不会依赖于对共享(静态)变量的更改。 但您最好查看一下您的 DBMS 是否支持诸如
INSERT OR UPDATE
之类的语句。将此功能委托给 DBMS 的另一个充分理由是,如果您需要部署多个应用程序服务器。 由于
同步
块不能跨机器工作,因此在这种情况下您无论如何都必须做其他事情。I think in this case, checkstyle is correct. In your code as presented, consider what would happen if two threads both had
row == null
at the entry to the synchronized block. Thread A would enter the block, and insert the new row. Then after thread A exits the block, thread B would enter the block (because it doesn't know what just happened), and try to insert the same new row again.I see you just changed the code and added a pretty important missing line in there. In the new code, you might be able to get away with that, since two threads won't be relying on changes to a shared (static) variable. But you might be better off seeing if your DBMS supports a statement such as
INSERT OR UPDATE
.Another good reason to delegate this functionality to the DBMS is if you ever need to deploy more than one application server. Since
synchronized
blocks don't work across machines, you will have to do something else in that case anyway.假设您希望读取最内层的行:
假设
dao().fetch
与 create 方法正确互斥,这不是一个经典的双重检查锁问题。编辑:(代码已更新)
双重检查锁的经典问题是在初始化发生之前分配一个值,其中两个线程正在访问相同的值。
假设 DAO 已正确同步并且不会返回部分初始化的值,则不会受到双重检查锁习惯用法的缺陷的影响。
Assuming you want that innermost line to read:
It's not a classic double-checked lock problem assuming
dao().fetch
is properly mutexed from the create method.Edit: (code was updated)
The classic problem of a double-checked lock is having a value assigned before initialization occurs where two threads are accessing the same value.
Assuming the DAO is properly synchronized and will not return a partially initialized value, this doesn't suffer from the flaws of the double-checked lock idiom.
如果您想编写这样的代码,请考虑:
自 Java 1.4 以来,同步方法已经变得相当便宜。 它不是免费的,但运行时确实不会受到太大影响,值得冒数据损坏的风险。
从 Java 1.5 开始,您就拥有了 Atomic* 类,它允许您以原子方式读取和设置字段。 不幸的是,他们不能解决你的问题。 为什么他们没有添加 AtomicCachedReference 或其他东西(当调用 get() 并且当前值 == null 时,它会调用可重写的方法)超出了我的范围。
尝试ehcache。 它允许您设置缓存(即,如果映射中不包含键,则允许您调用代码的对象)。 这通常是您想要的,并且缓存确实解决了您的问题(以及您甚至不知道它们存在的所有其他问题)。
If you're tempted to write code like this, consider:
Since Java 1.4, synchronizing methods has become pretty cheap. It's not free but the runtime really doesn't suffer that much that it's worthwhile to risk data corruption.
Since Java 1.5, you have the Atomic* classes which allow you to read and set fields in an atomic way. Unfortunately, they don't solve your problem. Why they didn't add AtomicCachedReference or something (which would call an overridable method when get() is called and the current value == null) is beyond me.
Try ehcache. It allows you to set up a cache (i.e. and object which allows you to call code if a key is not contained in a map). This is usually what you want and the caches really solve your problem (and all those other problems which you didn't know they even existed).
正如其他人指出的那样,这段代码将按原样执行您的意图,但仅在一组严格的非显而易见的假设下:
双重检查锁定习惯被破坏的原因(根据 Java 并发实践 的第 16.2.4 节)在进入同步块之前,运行此方法的线程可能会看到一个非空但初始化不正确的对“row”的引用(除非“dao”提供正确的同步)。 如果您的方法除了检查“行”是否为空之外还对“行”执行任何操作,那么它就会被破坏。 就目前情况而言,它可能没问题,但非常脆弱 - 就我个人而言,如果我认为其他开发人员稍后可能会修改该方法,那么我将不愿意提交此代码不了解 DCL 的微妙之处。
As others have pointed out, this code will do what you intend as is, but only under a strict set of non-obvious assumptions:
The reason the double-checked locking idiom is broken (per section 16.2.4 of Java Concurrency in Practice) is that it's possible for a thread running this method to see a non-null but improperly initialized reference to "row", before entering the synchronized block (unless "dao" provides proper synchronization). If your method were doing anything with "row" other than checking that it's null or not, it would be broken. As it stands, it is probably okay but very fragile - personally I wouldn't be comfortable committing this code if I thought there were even a remote chance that some other developer at some later time might modify the method without understanding the subtleties of DCL.