5

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? :)

4

4 に答える 4

5

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.

于 2008-12-01T06:56:15.393 に答える
4

Assuming you want that innermost line to read:

row = dao().create(id);

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.

于 2008-12-01T06:54:54.257 に答える
3

このようなコードを書きたくなった場合は、次のことを検討してください。

  • Java 1.4以降、メソッドの同期はかなり安価になりました。それは無料ではありませんが、ランタイムは実際にはデータ破壊のリスクを冒す価値があるほどそれほど苦しんでいません。

  • Java 1.5以降、アトミックな方法でフィールドを読み取ったり設定したりできるAtomic*クラスがあります。残念ながら、彼らはあなたの問題を解決しません。AtomicCachedReferenceなどを追加しなかった理由（get（）が呼び出され、現在の値== nullの場合にオーバーライド可能なメソッドを呼び出す）は私を超えています。

  • ehcacheを試してください。キャッシュ（つまり、キーがマップに含まれていない場合にコードを呼び出すことができるオブジェクト）を設定できます。これは通常あなたが望むものであり、キャッシュは本当にあなたの問題を解決します（そしてあなたがそれらが存在することさえ知らなかった他のすべての問題）。

于 2008-12-01T08:52:18.823 に答える
2

他の人が指摘したように、このコードは意図したとおりに動作しますが、明確でない仮定の厳密なセットの下でのみ実行されます。

  1. Java コードはクラスター化されていません (@Greg H の回答を参照)
  2. 「行」参照は、同期ブロックの前の最初の行でのみnull がチェックされます。

二重チェックのロックイディオムが壊れている理由 ( Java Concurrency in Practiceのセクション 16.2.4 による) は、このメソッドを実行しているスレッドが、null ではないが不適切に初期化された「行」への参照を参照する可能性があるためです。同期ブロック (「dao」が適切な同期を提供しない場合)。メソッドがnullかどうかをチェックする以外に「行」を使って何かをしていると、壊れてしまいます。現状では、おそらく問題ありませんが、非常に脆弱です。個人的には、他の開発者が後で DCL の微妙な点を理解せずにメソッドを変更する可能性がわずかにあると考えると、このコードをコミットすることに抵抗があります。

于 2008-12-02T18:53:39.080 に答える