From 31b58e17b1ba7fec76718f919952be3ce75578b9 Mon Sep 17 00:00:00 2001
From: evazion <noizave@gmail.com>
Date: Fri, 26 May 2017 16:22:03 -0500
Subject: [PATCH] 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.
---
 app/models/pool.rb | 16 ++++++++++------
 app/models/post.rb | 24 ++++++++++++++----------
 2 files changed, 24 insertions(+), 16 deletions(-)

diff --git a/app/models/pool.rb b/app/models/pool.rb
index e99eb910c..26fd0c5a9 100644
--- a/app/models/pool.rb
+++ b/app/models/pool.rb
@@ -246,9 +246,11 @@ class Pool < ActiveRecord::Base
     return if contains?(post.id)
     return if is_deleted?
 
-    update_attributes(:post_ids => add_number_to_string(post.id, post_ids), :post_count => post_count + 1)
-    post.add_pool!(self, true)
-    clear_post_id_array
+    with_lock do
+      update_attributes(:post_ids => add_number_to_string(post.id, post_ids), :post_count => post_count + 1)
+      post.add_pool!(self, true)
+      clear_post_id_array
+    end
   end
 
   def remove!(post)
@@ -256,9 +258,11 @@ class Pool < ActiveRecord::Base
     return unless CurrentUser.user.can_remove_from_pools?
     return if is_deleted?
 
-    update_attributes(:post_ids => remove_number_from_string(post.id, post_ids), :post_count => post_count - 1)
-    post.remove_pool!(self, true)
-    clear_post_id_array
+    with_lock do
+      update_attributes(:post_ids => remove_number_from_string(post.id, post_ids), :post_count => post_count - 1)
+      post.remove_pool!(self, true)
+      clear_post_id_array
+    end
   end
 
   def add_number_to_string(number, string)
diff --git a/app/models/post.rb b/app/models/post.rb
index 0edeb5ff5..ffc594d4e 100644
--- a/app/models/post.rb
+++ b/app/models/post.rb
@@ -1042,22 +1042,26 @@ class Post < ActiveRecord::Base
     def add_pool!(pool, force = false)
       return if belongs_to_pool?(pool)
       return if pool.is_deleted? && !force
-      reload
-      self.pool_string = "#{pool_string} pool:#{pool.id}".strip
-      set_pool_category_pseudo_tags
-      update_column(:pool_string, pool_string) unless new_record?
-      pool.add!(self)
+
+      with_lock do
+        self.pool_string = "#{pool_string} pool:#{pool.id}".strip
+        set_pool_category_pseudo_tags
+        update_column(:pool_string, pool_string) unless new_record?
+        pool.add!(self)
+      end
     end
 
     def remove_pool!(pool, force = false)
       return unless belongs_to_pool?(pool)
       return unless CurrentUser.user.can_remove_from_pools?
       return if pool.is_deleted? && !force
-      reload
-      self.pool_string = pool_string.gsub(/(?:\A| )pool:#{pool.id}(?:\Z| )/, " ").strip
-      set_pool_category_pseudo_tags
-      update_column(:pool_string, pool_string) unless new_record?
-      pool.remove!(self)
+
+      with_lock do
+        self.pool_string = pool_string.gsub(/(?:\A| )pool:#{pool.id}(?:\Z| )/, " ").strip
+        set_pool_category_pseudo_tags
+        update_column(:pool_string, pool_string) unless new_record?
+        pool.remove!(self)
+      end
     end
 
     def remove_from_all_pools