次のコードのレビューにご協力ください。スレッドセーフですか?

https://stackoverflow.com/questions/934130

06-09-2019
|

質問

private static Callback callback;

public Foo()
{
    super(getCallback());
}

private static Callback getCallback()
{
    callback = new Callback();
    return callback;
}

コンストラクター Foo() は、複数のスレッドから呼び出される可能性があります。私が懸念しているのは、プライベート静的フィールド「callback」と静的メソッド「getCallback()」です。

見てわかるように、「getCallback()」が呼び出されるたびに、静的フィールド「callback」に新しい値が割り当てられます。

私の推測キーワードがあるためスレッドセーフではないということです静的つまり、Foo の静的フィールド「コールバック」は、別の Foo() を構築している他のスレッドによって上書きされる可能性があります。これは正しいです？

間違っていたら訂正してください。ありがとう！

編集：私の目的は、後で再利用できるように、クラス内のどこかに「コールバック」を保持することです。しかし、これは簡単ではありません。Foo は、「コールバック」を渡すことを義務付けるコンストラクターを持つクラスから拡張されているからです。

解決

はい、あなたは正しいです。 Fooの2つのインスタンスが同じCallBackインスタンスで終わるするために2つのスレッドが同時にgetCallback()方法を入力し、他方はすでにこれを行ったが、まだ戻っていない間、1が静的フィールドに新しいCallBackを割り当てたときにそれは可能です。それが何の目的を果たしていないので、この場合には、最良の修正は、静的フィールドを持つことはありません。また、getCallback()が同期します。

しかし、それはのないの真のスレッドセーフでないコードでのみstaticのキーワードの結果ということだと注意します。

他のヒント

これは、スレッドセーフではありません。これらの代替案を試してください：

オプション1：ここでのすべてのインスタンスが同じコールバックを共有

private static final Callback callback = new Callback();

public Foo() {
    super(callback);
}

オプション2：ここでは、各インスタンスは、独自のコールバックを持っている。

public Foo() {
    super(new Callback());
}

コンストラクタは、スレッドセーフですが、クラス全体のスレッドの安全性は、コールバックの実装に依存し、両方のケースであることに注意してください。それが変更可能な状態を持っている場合は、潜在的な問題があるでしょう。コールバックが不変である場合は、スレッドセーフを持っています。

コールバックは、新しい値ごとに単一の時間はFoo（）（でも、同じスレッドから）と呼ばれているを取得します。私はあなたのコードが実行すべきか非常にわからないんだけど（あなたは一度だけ（シングルトン）静的変数を初期化したい場合にはgetCallback（まだnullの場合、あなたが）確認する必要があります - とactionCallbackは何ですか？）。それはスレッドセーフ作りのために、同期使用します。

私は、あなたが完全にそれを自分でまとめたと思うが、あなたが達成しようとしているかについての詳細なしで、あなたの問題を解決するための提案をすることが難しいことでしょう。

1つの明白な疑問は、callbackは静的でなければならないのですか？それとも、安全にあなたのクラスの機能を壊すことなく、それインスタンスフィールド作ることができる？

答えが出ていることはわかっていますが、その理由は詳しく説明されていません。

2 つのスレッドが getCallback() メソッドを呼び出している場合、次のように行を実行できます。

スレッド 1 - コールバック = new Callback();
スレッド 2 - コールバック = new Callback();
スレッド 1 - actionCallback を返します。
スレッド 2 - actionCallback を返します。

この場合、(2.) で生成されたコールバックは (3.) と (4.) の両方で返されます。

解決策は、コールバックがクラスではなくインスタンスに固有である場合、なぜ静的に定義されたコールバックなのかを尋ねることのようです。

それが役立つことを願っています。

あなたは、しかし、あなたがそれを必要とする場合は、次の操作を行うことができた場合、その一般的に良いアイデアは、このパターンを回避するために、なぜあなたは見つけることができます検索を行う場合は、

何をしようとしていることは、Singletonパターンと呼ばれています。

private static final Callback CALLBACK= new Callback();

それとも、怠惰なシングルトンが必要な場合はあなたが行うことができます。

public class Foo {
   class CallbackHolder {
       static final Callback CALLBACK= new Callback();
   }

   public static Callback getCallback() {
      return CallbackHolder.CALLBACK;
   }

public Foo() {
    super(getCallback());
}

どちらの実装は、スレッドセーフです。

あなたは1つのスレッドあたりのコールバック、オブジェクトごとに、または真のシングルトンをしたいですか？

異なる変形を行う方法についていくつかのスケッチ - ちょうど私の頭の上からは、あまりにも文字通りこれらを取ることはありません。）

私はコールバックは、それはあなたがこれらのすべてに多くを簡素化することができますささいなコンストラクタの場合は、処理する必要のある例外をスローする可能性があります非自明なコンストラクタを持っていると仮定してきたことに注意してください。

スレッドごとに1つます：

  private static ThreadLocal<Callback> callback;

  public Foo()
  {
      super(getCallback());
  }

  private static Callback getCallback()
  {
      if ( callback.get() == null ) 
          callback.set(new Callback());
      return callback.get();
  }

すべてのスレッドのシングルコールバックます：

  private final static Callback callback;

  static {
      callback = new Callback(); 
  }

  public Foo()
  {
      super(getCallback());
  }

  private static Callback getCallback()
  {
      return callback;
  }

そして、completnessため、オブジェクトごとに1つのコールバックます：

  private Callback callback;

  public Foo()
  {
      super(getCallback());
  }

  private Callback getCallback()
  {
      callback = new Callback();
      return callback;
  }

ライセンス： CC-BY-SA と帰属

所属していません StackOverflow

次のコードのレビューにご協力ください。スレッド セーフですか?

次のコードのレビューにご協力ください。スレッドセーフですか?