Inizializzazione lazy non thread-safe in PostActionType

Bug
1

Durante l’utilizzo di discourse in un’implementazione Ruby multithread e server (TruffleRuby/Puma), gli errori sono prodotti dall’uso non sicuro degli hash nelle impostazioni dei flag e questo pattern di inizializzazione pigra deve evitare di assegnare due volte:

3 Mi Piace
2

Una semplice chiamata a PostActionType.flag_settings in un inizializzatore risolve il problema? (ad esempio: inserisci qui: discourse/config/initializers/000-mini_sql.rb at 3e0cb8ea47ea27cb3b564ac10656884739e4d78c · discourse/discourse · GitHub temporaneamente)

Suppongo che potremmo sincronizzare anche questo blocco. Ci sono altre grandi bandiere rosse che truffle sta sollevando?

2 Mi Piace
3

Sì, dovrebbe funzionare.
C’è qualche motivo per inizializzarlo in modo lazy?
Inizializzarlo eager eviterebbe qualsiasi problema di sincronizzazione e sarebbe leggermente più veloce da accedere, oltre che più semplice.

1 Mi Piace
4

Onestamente non sono sicuro che questa inizializzazione pigra abbia senso anche qui, abbiamo un hook per i plugin per sostituire l’intero set di flag, ma questo verrebbe attivato prima dell’inizializzatore.

@roman hai qualche preoccupazione riguardo a un’inizializzazione anticipata qui? Suppongo che la soluzione banale sia:

class << self

  def flag_settings
     ...
   end
end

flag_settings

Sto pensando che il motivo principale per cui è successo questo è che Ruby non fornisce uno schema pulito per gli inizializzatori di classe, quindi spinge gli sviluppatori a inventarne di propri/usare quelli pigri.

2 Mi Piace
5

Potrebbe anche essere fatto in questo modo, che eviterebbe in particolare di leggere l’ivar due volte e renderebbe più chiaro che è inizializzato in modo eager (la logica potrebbe anche essere in un metodo helper initial_flag_settings per l’organizzazione, ovviamente):

  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 Mi Piace
6

Sì, penso che l’inizializzazione anticipata sarebbe sicura qui. Inoltre, non dovrebbe entrare in conflitto con l’API dei plugin, che viene raramente utilizzata.

3 Mi Piace
7

Ecco una PR:

4 Mi Piace
8

Questo argomento è stato chiuso automaticamente dopo 4 giorni. Non sono più consentite nuove risposte.