Aide à la révision du code suivant, est-il sûr pour les threads?

Question

private static Callback callback;

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

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

Le constructeur Foo() peut potentiellement être appelé à partir de plusieurs threads. Mon inquiétude concerne le champ statique privé 'callback' et la méthode statique 'getCallback()'.

Comme on peut le voir, à chaque fois que 'getCallback()' est appelé, il assigne une nouvelle valeur au champ statique 'callback'.

Je pense que ce n'est pas thread-safe car le mot-clé static est toujours attaché à la classe et non à l'instance, ce qui signifie que le champ statique 'callback' d'un Foo peut potentiellement être écrasé par un autre thread qui construit un autre Foo(). Est-ce correct?

Veuillez me corriger si je me trompe. Merci!

EDIT: Mon intention est de conserver 'callback' quelque part dans la classe, afin de pouvoir le réutiliser plus tard. Mais ce n'est pas facile car Foo étend une classe dont le constructeur exige que 'callback' soit passé.

Answer 1

Ce que vous essayez de faire s'appelle un modèle Singleton, si vous effectuez une recherche, vous pouvez découvrir pourquoi il est généralement une bonne idée d'éviter ce modèle si vous le pouvez, cependant 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, 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 2

Voulez-vous un rappel par fil, un par objet, ou un Singleton vrai?

Quelques croquis sur comment faire les différentes variantes - juste de mémoire, ne les prenez pas trop littéralement :)

Veuillez noter que j'ai supposé que le Callback a un constructeur non trivial qui peut générer des exceptions qui doivent être gérées, si c'est un constructeur trivial, vous pouvez simplifier tout cela beaucoup.

Un par fil:

  private static ThreadLocal callback;

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

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

Un seul rappel pour tous les fils:

  private final static Callback callback;

  static {
      callback = new Callback(); 
  }

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

  private static Callback getCallback()
  {
      return callback;
  }

Et, pour compléter, un rappel par objet:

  private Callback callback;

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

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