From 0bd7f4ad44c53832ec029a53a88461e9f6eab6f1 Mon Sep 17 00:00:00 2001
From: Guo Xiang Tan <tgx_world@hotmail.com>
Date: Tue, 21 Nov 2017 13:20:15 +0800
Subject: Raise an error if advisory lock in migrator was not released.

---
 activerecord/lib/active_record/migration.rb | 10 ++++++++--
 activerecord/test/cases/migration_test.rb   | 19 +++++++++++++++++++
 2 files changed, 27 insertions(+), 2 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/migration.rb b/activerecord/lib/active_record/migration.rb
index 360bf25a8c..ecf74b9824 100644
--- a/activerecord/lib/active_record/migration.rb
+++ b/activerecord/lib/active_record/migration.rb
@@ -140,6 +140,7 @@ module ActiveRecord
 
   class ConcurrentMigrationError < MigrationError #:nodoc:
     DEFAULT_MESSAGE = "Cannot run migrations because another migration process is currently running.".freeze
+    RELEASE_LOCK_FAILED_MESSAGE = "Failed to release advisory lock".freeze
 
     def initialize(message = DEFAULT_MESSAGE)
       super
@@ -1337,12 +1338,17 @@ module ActiveRecord
 
       def with_advisory_lock
         lock_id = generate_migrator_advisory_lock_id
-        got_lock = Base.connection.get_advisory_lock(lock_id)
+        connection = Base.connection
+        got_lock = connection.get_advisory_lock(lock_id)
         raise ConcurrentMigrationError unless got_lock
         load_migrated # reload schema_migrations to be sure it wasn't changed by another process before we got the lock
         yield
       ensure
-        Base.connection.release_advisory_lock(lock_id) if got_lock
+        if got_lock && !connection.release_advisory_lock(lock_id)
+          raise ConcurrentMigrationError.new(
+            ConcurrentMigrationError::RELEASE_LOCK_FAILED_MESSAGE
+          )
+        end
       end
 
       MIGRATOR_SALT = 2053462845
diff --git a/activerecord/test/cases/migration_test.rb b/activerecord/test/cases/migration_test.rb
index b18af2ab55..85107c26d3 100644
--- a/activerecord/test/cases/migration_test.rb
+++ b/activerecord/test/cases/migration_test.rb
@@ -678,6 +678,25 @@ class MigrationTest < ActiveRecord::TestCase
       assert_no_column Person, :last_name,
         "without an advisory lock, the Migrator should not make any changes, but it did."
     end
+
+    def test_with_advisory_lock_raises_the_right_error_when_it_fails_to_release_lock
+      migration = Class.new(ActiveRecord::Migration::Current).new
+      migrator = ActiveRecord::Migrator.new(:up, [migration], 100)
+      lock_id = migrator.send(:generate_migrator_advisory_lock_id)
+
+      e = assert_raises(ActiveRecord::ConcurrentMigrationError) do
+        silence_stream($stderr) do
+          migrator.send(:with_advisory_lock) do
+            ActiveRecord::Base.connection.release_advisory_lock(lock_id)
+          end
+        end
+      end
+
+      assert_match(
+        /#{ActiveRecord::ConcurrentMigrationError::RELEASE_LOCK_FAILED_MESSAGE}/,
+        e.message
+      )
+    end
   end
 
   private
-- 
cgit v1.2.3