Initialisation paresseuse non thread-safe dans PostActionType

Bug
1

Lors de l’utilisation de Discourse dans une implémentation Ruby multithread et un serveur (TruffleRuby/Puma), des erreurs sont produites par l’utilisation non sécurisée des hachages dans les paramètres de drapeaux et ce modèle d’initialisation paresseuse doit éviter les affectations doubles :

3 « J'aime »
2

Un simple appel à PostActionType.flag_settings dans un initialiseur résout-il le problème ? (par exemple, insérez simplement ici : discourse/config/initializers/000-mini_sql.rb at 3e0cb8ea47ea27cb3b564ac10656884739e4d78c · discourse/discourse · GitHub temporairement)

Je suppose que nous pourrions également synchroniser ce bloc. Y a-t-il d’autres drapeaux rouges majeurs que truffle soulève ?

2 « J'aime »
3

Oui, cela devrait fonctionner.
Y a-t-il une raison d’initialiser cela de manière paresseuse ?
L’initialiser de manière anticipée éviterait tout problème de synchronisation et serait légèrement plus rapide d’accès, ainsi que plus simple.

1 « J'aime »
4

Honnêtement, je ne suis pas sûr que cette initialisation paresseuse ait un sens ici non plus, nous avons un hook pour que les plugins remplacent l’ensemble des drapeaux, mais cela se déclencherait avant l’initialiseur.

@roman, des préoccupations concernant une initialisation anticipée ici ? Je suppose que la correction triviale est :

class << self

  def flag_settings
     ...
   end
end

flag_settings

Je pense que la principale raison pour laquelle cela s’est produit est que Ruby n’offre pas de modèle propre pour les initialiseurs de classe, ce qui pousse les développeurs à en inventer des propres/à utiliser l’initialisation paresseuse.

2 « J'aime »
5

Cela pourrait aussi être fait ainsi, ce qui éviterait notamment de lire l’ivar deux fois et rendrait plus claire son initialisation anticipée (la logique pourrait aussi être dans une méthode d’aide initial_flag_settings pour l’organisation, bien sûr) :

  class << self
    attr_reader :flag_settings
  end

  @flag_settings = FlagSettings.new
  @flag_settings.add(
    3,
    :off_topic,
    notify_type: true,
    auto_action_type: true,
  )
  # ...
2 « J'aime »
6

Oui, je pense que l’initialisation anticipée serait sûre ici. Elle ne devrait pas non plus entrer en conflit avec l’API des plugins, qui est rarement utilisée.

3 « J'aime »
7

Voici une PR :

4 « J'aime »
8

Ce sujet a été automatiquement fermé après 4 jours. Les nouvelles réponses ne sont plus autorisées.