fix: raise_lock_acquisition_error if the job cannot set the lock (#8744)

Consider a scenario where two jobs are concurrently accessing a job with a lock mechanism. If the second job accesses lock_manager.locked? before the first job called lock_manager.lock(lock_key), it would return a false positive. At this point, both jobs can be executed freely.

To address this issue, the following change ensures that only the current thread that has set the lock can execute. Otherwise, a LockAcquisitionError will be thrown.

Co-authored-by: Sojan Jose <sojan@pepalo.com>
Co-authored-by: Shivam Mishra <scm.mymail@gmail.com>

app/jobs/mutex_application_job.rb

@@ -18,17 +18,15 @@ class MutexApplicationJob < ApplicationJob
     lock_key = format(key_format, *args)
     lock_manager = Redis::LockManager.new
 
-    if lock_manager.locked?(lock_key)
-      Rails.logger.warn "[#{self.class.name}] Failed to acquire lock on attempt #{executions}: #{lock_key}"
-      raise LockAcquisitionError, "Failed to acquire lock for key: #{lock_key}"
-    end
-
     begin
-      lock_manager.lock(lock_key)
-      Rails.logger.info "[#{self.class.name}] Acquired lock for: #{lock_key} on attempt #{executions}"
-      yield
+      if lock_manager.lock(lock_key)
+        Rails.logger.info "[#{self.class.name}] Acquired lock for: #{lock_key} on attempt #{executions}"
+        yield
+      else
+        Rails.logger.warn "[#{self.class.name}] Failed to acquire lock on attempt #{executions}: #{lock_key}"
+        raise LockAcquisitionError, "Failed to acquire lock for key: #{lock_key}"
+      end
     ensure
-      # Ensure that the lock is released even if there's an error in processing
       lock_manager.unlock(lock_key)
     end
   end

spec/jobs/mutex_application_job_spec.rb

@@ -16,14 +16,12 @@ RSpec.describe MutexApplicationJob do
 
   before do
     allow(Redis::LockManager).to receive(:new).and_return(lock_manager)
-    allow(lock_manager).to receive(:locked?).and_return(false)
     allow(lock_manager).to receive(:lock).and_return(true)
     allow(lock_manager).to receive(:unlock).and_return(true)
   end
 
   describe '#with_lock' do
     it 'acquires the lock and yields the block if lock is not acquired' do
-      expect(lock_manager).to receive(:locked?).with(lock_key).and_return(false)
       expect(lock_manager).to receive(:lock).with(lock_key).and_return(true)
       expect(lock_manager).to receive(:unlock).with(lock_key).and_return(true)
 
@@ -31,7 +29,7 @@ RSpec.describe MutexApplicationJob do
     end
 
     it 'raises LockAcquisitionError if it cannot acquire the lock' do
-      allow(lock_manager).to receive(:locked?).with(lock_key).and_return(true)
+      allow(lock_manager).to receive(:lock).with(lock_key).and_return(false)
 
       expect do
         described_class.new.send(:with_lock, lock_key) do
@@ -41,7 +39,6 @@ RSpec.describe MutexApplicationJob do
     end
 
     it 'ensures that the lock is released even if there is an error during block execution' do
-      expect(lock_manager).to receive(:locked?).with(lock_key).and_return(false)
       expect(lock_manager).to receive(:lock).with(lock_key).and_return(true)
       expect(lock_manager).to receive(:unlock).with(lock_key).and_return(true)