pools: lock pool when adding/removing posts (fixes #3091).

Adding a post id to a pool's post_ids string is non-atomic, hence we
must lock the pool to avoid a race condition.

Adding a pool to a post's pool_string is likewise non-atomic, hence we
must lock the post as well.
This commit is contained in:
evazion
2017-05-26 16:22:03 -05:00
parent f38810bd1c
commit 31b58e17b1
2 changed files with 24 additions and 16 deletions

View File

@@ -246,20 +246,24 @@ class Pool < ActiveRecord::Base
return if contains?(post.id) return if contains?(post.id)
return if is_deleted? return if is_deleted?
with_lock do
update_attributes(:post_ids => add_number_to_string(post.id, post_ids), :post_count => post_count + 1) update_attributes(:post_ids => add_number_to_string(post.id, post_ids), :post_count => post_count + 1)
post.add_pool!(self, true) post.add_pool!(self, true)
clear_post_id_array clear_post_id_array
end end
end
def remove!(post) def remove!(post)
return unless contains?(post.id) return unless contains?(post.id)
return unless CurrentUser.user.can_remove_from_pools? return unless CurrentUser.user.can_remove_from_pools?
return if is_deleted? return if is_deleted?
with_lock do
update_attributes(:post_ids => remove_number_from_string(post.id, post_ids), :post_count => post_count - 1) update_attributes(:post_ids => remove_number_from_string(post.id, post_ids), :post_count => post_count - 1)
post.remove_pool!(self, true) post.remove_pool!(self, true)
clear_post_id_array clear_post_id_array
end end
end
def add_number_to_string(number, string) def add_number_to_string(number, string)
"#{string} #{number}" "#{string} #{number}"

View File

@@ -1042,23 +1042,27 @@ class Post < ActiveRecord::Base
def add_pool!(pool, force = false) def add_pool!(pool, force = false)
return if belongs_to_pool?(pool) return if belongs_to_pool?(pool)
return if pool.is_deleted? && !force return if pool.is_deleted? && !force
reload
with_lock do
self.pool_string = "#{pool_string} pool:#{pool.id}".strip self.pool_string = "#{pool_string} pool:#{pool.id}".strip
set_pool_category_pseudo_tags set_pool_category_pseudo_tags
update_column(:pool_string, pool_string) unless new_record? update_column(:pool_string, pool_string) unless new_record?
pool.add!(self) pool.add!(self)
end end
end
def remove_pool!(pool, force = false) def remove_pool!(pool, force = false)
return unless belongs_to_pool?(pool) return unless belongs_to_pool?(pool)
return unless CurrentUser.user.can_remove_from_pools? return unless CurrentUser.user.can_remove_from_pools?
return if pool.is_deleted? && !force return if pool.is_deleted? && !force
reload
with_lock do
self.pool_string = pool_string.gsub(/(?:\A| )pool:#{pool.id}(?:\Z| )/, " ").strip self.pool_string = pool_string.gsub(/(?:\A| )pool:#{pool.id}(?:\Z| )/, " ").strip
set_pool_category_pseudo_tags set_pool_category_pseudo_tags
update_column(:pool_string, pool_string) unless new_record? update_column(:pool_string, pool_string) unless new_record?
pool.remove!(self) pool.remove!(self)
end end
end
def remove_from_all_pools def remove_from_all_pools
pools.find_each do |pool| pools.find_each do |pool|