Inicialização preguiçosa não segura para threads em PostActionType

Bug
1

Ao usar o discourse em uma implementação Ruby multithread e servidor (TruffleRuby/Puma), erros são produzidos pelo uso inseguro de hash nas configurações de flags e este padrão de inicialização preguiçosa precisa evitar atribuições duplas:

3 curtidas
2

Uma chamada simples para PostActionType.flag_settings em um inicializador resolve o problema? (por exemplo: basta inserir aqui: discourse/config/initializers/000-mini_sql.rb at 3e0cb8ea47ea27cb3b564ac10656884739e4d78c · discourse/discourse · GitHub temporariamente)

Eu acho que poderíamos sincronizar este bloco também. Existem outras grandes bandeiras vermelhas que o truffle está levantando?

2 curtidas
3

Sim, isso deve funcionar.
Existe alguma razão para inicializar isso de forma preguiçosa?
Inicializá-lo de forma ansiosa evitaria qualquer problema de sincronização e seria um pouco mais rápido de acessar, além de mais simples.

1 curtida
4

Sinceramente, não tenho certeza se essa inicialização preguiçosa faz sentido aqui também, temos um hook para plugins substituírem todo o conjunto de flags, mas isso aconteceria antes do inicializador.

@roman, alguma preocupação com uma inicialização antecipada aqui? Acho que a correção trivial é:

class << self

  def flag_settings
     ...
   end
end

flag_settings

Estou pensando que o grande motivo pelo qual isso aconteceu é que o Ruby não oferece um padrão limpo para inicializadores de classe, então ele incentiva os desenvolvedores a inventarem os seus próprios/usarem inicialização preguiçosa.

2 curtidas
5

Também poderia ser feito assim, o que notavelmente evitaria ler o ivar duas vezes e deixaria mais claro que ele é inicializado antecipadamente (a lógica também poderia estar em um método auxiliar initial_flag_settings para organização, é claro):

  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 curtidas
6

Sim, acho que a inicialização antecipada seria segura aqui. Também não deve entrar em conflito com a API de plugins, que raramente é usada.

3 curtidas
7

Aqui está um PR:

4 curtidas
8

Este tópico foi fechado automaticamente após 4 dias. Novas respostas não são mais permitidas.