Is Zammad thread safe when using Puma?

[josefsmvm]

Hi All,
Looking at the Zammad code I see several models using class variables, which are not thread safe.
If the application server is not multi thread I think that there is no problem, but if Puma is used as the application server could occur a race condition, when two or more threads try to access the same class variable, am I correct? If not, what am I missing out on this?

Thanks,

[thorsteneckel]

Hi @josefsmvm - can you give a practical example? We are not aware of any race conditions.

[josefsmvm]

Hi @thorsteneckel, yes I can give you an example. I pulled Zammad from github, master branch, and used the Scheduler model to do test.

Scheduler uses the class variable @@jobs_started to keep track of the jobs. In non multithread environment I think there is no problem because this variable is exclusive to the process started by the application server. But Puma starts a new thread for each request in the same process, so the variable is shared among them. To simulate that behaviour I started a rails console and used the below code:

Thread.new { jobs = Scheduler.class_variable_get(:@@jobs_started); puts "Thread1: #{jobs.object_id}"; jobs[1] = '11'; puts "Thread1: #{jobs[1]}"; sleep 3; puts "Thread1: #{jobs[1]}" }; Thread.new { sleep 1; jobs = Scheduler.class_variable_get(:@@jobs_started); puts "Thread2: #{jobs.object_id}"; jobs[1] = '22'; puts "Thread2: #{jobs[1]}" }

The result was:

Thead1: 47318324424760
Thead1: 11
Thead2: 47318324424760
Thead2: 22
Thead1: 22

We can see that the object id of the variable is the same, thread 1 changes the hash to 11 and sleeps to allow thread 2 to change it to 22, at the end thread 1 puts the hash value and it was changed to 22. So, the variable is shared among threads in the same process. I didn’t test other models, but I see class variables in Setting and Channel.

Hash is also not thread safe, and the variable is a hash. I don’t know what would happen if the two codes below, extracted from Scheduler.rb, were executed at the same time in two threads:

def self.threads
  .....
  @@jobs_started[ job.id ] = start_job(job)  <= This code in one thread
  .....
end

def self.skip_job?(job)
    thread = @@jobs_started[ job.id ]
    return false if thread.blank?

    # check for validity of thread instance
    if !thread.respond_to?(:status)
      logger.error "Invalid thread stored for job '#{job.name}' (#{job.method}): #{thread.inspect}. Deleting and resting job."
      @@jobs_started.delete(job.id)  <= This code in another thread
      return false
    end
  ...
end

This is the reason why I put this question. I think it is very difficult to identify this kind of situation in production. Please, correct me if I am missing something.

Thanks,

[thorsteneckel]

Hi @josefsmvm - you’re right about this. At least partially: These wouldn’t be thread safe. However, the scheduler is running in a separate process - outside of puma. These variables are on purpose not thread safe (to manage the running jobs by the scheduler dispatcher).