Aide en revue le code suivant, est-ce thread-safe?

Question

private static Callback callback;

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

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

Constructor Foo () peut éventuellement être appelée à partir de plusieurs threads. Ce qui me préoccupe est le champ statique privé « rappel » et la méthode statique 'getCallback ().

Comme on peut le voir, chaque fois que «getCallback () est appelée, elle attribue une nouvelle valeur à champ statique « rappel ».

My guess est qu'il est pas sûre parce que le mot-clé statique est toujours attaché à la classe ne l'instance, de sorte que des moyens, le champ statique « de rappel » d'un Foo peut potentiellement être remplacé par un autre thread qui construit une autre Foo (). Est-ce exact?

S'il vous plaît me corriger si je me trompe. Merci!

EDIT: Mon intention est de garder « rappel » quelque part dans la classe, donc je peux le réutiliser plus tard. Mais ce n'est pas facile parce que Foo s'étend d'une classe qui a mandatement constructeur « de rappel » à transmettre.

Answer 1

Oui, vous avez raison. Il est possible pour deux instances de Foo pour se retrouver avec la même instance de CallBack lorsque deux threads entrent dans la méthode getCallback() simultanément et on assigne une nouvelle CallBack au champ statique tandis que l'autre a déjà fait, mais pas encore de retour. Dans ce cas, la meilleure solution est de ne pas avoir le champ statique, car il ne sert à rien. Vous pouvez également faire getCallback() synchronisé.

Mais notez qu'il est pas vrai que seuls les résultats par mot-clé static dans le code qui ne sont pas thread-safe.

Answer 2

Il est pas thread-safe. Essayez ces alternatives:

Option 1: ici toutes les instances partagent le même rappel

private static final Callback callback = new Callback();

public Foo() {
    super(callback);
}

Option 2: ici chaque instance a son propre rappel

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

Notez que dans les deux cas, bien que le constructeur est thread-safe, le fil de sécurité de toute la classe dépend de la mise en œuvre de rappel. Si elle a l'état mutable, alors vous aurez des problèmes potentiels. Si Callback est immuable, alors vous avez la sécurité des threads.

Answer 3

callback obtenir une nouvelle valeur à chaque fois Foo () est appelée (même du même fil). Je ne suis pas tout à fait sûr de ce que votre code devrait faire (si vous voulez initialiser la variable statique une seule fois (singleton), vous devriez vérifier si elle est encore nulle dans getCallback () - et ce qui est actionCallback?). Pour rendre thread-safe, utilisez synchronisé.

Answer 4

Je pense que vous le résuma vous parfaitement, mais sans plus de détails sur ce que vous essayez d'atteindre, il sera difficile de faire des suggestions pour résoudre votre problème.

Une question évidente est, ne callback doivent être statique? Ou pourriez-vous faire en toute sécurité un champ d'instance sans casser la fonctionnalité de votre classe?

Answer 5

Je sais qu'il a été répondu, mais le pourquoi n'a pas vraiment été détaillé.

deux fils appellent la méthode getCallback (), ils pourraient exécuter les lignes comme suit:

1. Discussion 1 - rappel = new Callback ();
2. Discussion 2 - rappel = new Callback ();
3. Discussion 1 - return actionCallback;
4. Discussion 2 - retour actionCallback;

Dans ce cas, le rappel généré à (2.) seraient renvoyés dans les deux (3) et (4).

La solution semble être de se demander pourquoi le rappel défini statiquement si elle est spécifique à l'instance ne classe.

J'espère que cela aide.

Answer 6

Qu'est-ce que vous essayez de faire est appelé un modèle Singleton, si vous effectuez une recherche, vous pouvez savoir pourquoi il est généralement une bonne idée d'éviter ce modèle si vous pouvez, si vous en avez besoin, vous pouvez faire ce qui suit.

private static final Callback CALLBACK= new Callback();

Ou si vous avez besoin d'un Singleton paresseux que vous pouvez faire

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

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

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

Les deux implémentations sont thread-safe.

Answer 7

Voulez-vous un rappel par thread, un par un objet ou d'un vrai Singleton?

Quelques croquis sur la façon de faire les différentes variantes - juste du haut de ma tête, ne prennent pas ces trop littéralement:)

S'il vous plaît noter que je suis supposé que le rappel a un constructeur non négligeable qui pourrait jeter exception qui doit être traitée, si elle est un constructeur trivial vous pouvez simplifier tout cela beaucoup.

Un par fil:

  private static ThreadLocal<Callback> callback;

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

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

rappel unique pour tous les sujets:

  private final static Callback callback;

  static {
      callback = new Callback(); 
  }

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

  private static Callback getCallback()
  {
      return callback;
  }

Et, pour complétude, un rappel par l'objet:

  private Callback callback;

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

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