From 993af5d0d2d596040195c4109c531c33deeac026 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Tue, 11 Jun 2013 18:15:18 +0300 Subject: [PATCH] cattr_accessor is not threadsafe! --- app/controllers/application_controller.rb | 7 +++---- app/observers/base_observer.rb | 4 ++++ app/observers/issue_observer.rb | 2 -- app/observers/merge_request_observer.rb | 2 -- config/database.yml.mysql | 2 +- config/database.yml.postgresql | 2 +- lib/api/issues.rb | 3 ++- lib/api/merge_requests.rb | 3 +-- spec/models/milestone_spec.rb | 1 - 9 files changed, 12 insertions(+), 14 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 27b49e1b3cf..09af5b94164 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -1,7 +1,7 @@ class ApplicationController < ActionController::Base before_filter :authenticate_user! before_filter :reject_blocked! - before_filter :set_current_user_for_observers + before_filter :set_current_user_for_thread before_filter :add_abilities before_filter :dev_tools if Rails.env == 'development' before_filter :default_headers @@ -47,9 +47,8 @@ class ApplicationController < ActionController::Base end end - def set_current_user_for_observers - MergeRequestObserver.current_user = current_user - IssueObserver.current_user = current_user + def set_current_user_for_thread + Thread.current[:current_user] = current_user end def abilities diff --git a/app/observers/base_observer.rb b/app/observers/base_observer.rb index 182d3b7b73c..92b73981d27 100644 --- a/app/observers/base_observer.rb +++ b/app/observers/base_observer.rb @@ -6,4 +6,8 @@ class BaseObserver < ActiveRecord::Observer def log_info message Gitlab::AppLogger.info message end + + def current_user + Thread.current[:current_user] + end end diff --git a/app/observers/issue_observer.rb b/app/observers/issue_observer.rb index 03ce4b95ac8..888fa7f6b73 100644 --- a/app/observers/issue_observer.rb +++ b/app/observers/issue_observer.rb @@ -1,6 +1,4 @@ class IssueObserver < BaseObserver - cattr_accessor :current_user - def after_create(issue) notification.new_issue(issue, current_user) end diff --git a/app/observers/merge_request_observer.rb b/app/observers/merge_request_observer.rb index d0dfad8869d..03d4a22c1e6 100644 --- a/app/observers/merge_request_observer.rb +++ b/app/observers/merge_request_observer.rb @@ -1,6 +1,4 @@ class MergeRequestObserver < BaseObserver - cattr_accessor :current_user - def after_create(merge_request) notification.new_merge_request(merge_request, current_user) end diff --git a/config/database.yml.mysql b/config/database.yml.mysql index 436bea77f0f..a3eff1a74f8 100644 --- a/config/database.yml.mysql +++ b/config/database.yml.mysql @@ -6,7 +6,7 @@ production: encoding: utf8 reconnect: false database: gitlabhq_production - pool: 5 + pool: 10 username: root password: "secure password" # host: localhost diff --git a/config/database.yml.postgresql b/config/database.yml.postgresql index 4a4aa3465a6..4b74f3348f8 100644 --- a/config/database.yml.postgresql +++ b/config/database.yml.postgresql @@ -5,7 +5,7 @@ production: adapter: postgresql encoding: unicode database: gitlabhq_production - pool: 5 + pool: 10 username: git password: # host: localhost diff --git a/lib/api/issues.rb b/lib/api/issues.rb index a2983e197e7..a15203d1563 100644 --- a/lib/api/issues.rb +++ b/lib/api/issues.rb @@ -2,6 +2,7 @@ module API # Issues API class Issues < Grape::API before { authenticate! } + before { Thread.current[:current_user] = current_user } resource :issues do # Get currently authenticated user's issues @@ -79,7 +80,7 @@ module API attrs = attributes_for_keys [:title, :description, :assignee_id, :milestone_id, :state_event] attrs[:label_list] = params[:labels] if params[:labels].present? - IssueObserver.current_user = current_user + if @issue.update_attributes attrs present @issue, with: Entities::Issue else diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb index 23e2f82889f..861a4f4d159 100644 --- a/lib/api/merge_requests.rb +++ b/lib/api/merge_requests.rb @@ -2,6 +2,7 @@ module API # MergeRequest API class MergeRequests < Grape::API before { authenticate! } + before { Thread.current[:current_user] = current_user } resource :projects do helpers do @@ -94,8 +95,6 @@ module API authorize! :modify_merge_request, merge_request - MergeRequestObserver.current_user = current_user - if merge_request.update_attributes attrs merge_request.reload_code merge_request.mark_as_unchecked diff --git a/spec/models/milestone_spec.rb b/spec/models/milestone_spec.rb index efab5510c59..d2819b7c1d6 100644 --- a/spec/models/milestone_spec.rb +++ b/spec/models/milestone_spec.rb @@ -39,7 +39,6 @@ describe Milestone do end it "should count closed issues" do - IssueObserver.current_user = issue.author issue.close milestone.issues << issue milestone.percent_complete.should == 100