diff --git a/lib/concurrent-ruby/concurrent/atomic/reentrant_read_write_lock.rb b/lib/concurrent-ruby/concurrent/atomic/reentrant_read_write_lock.rb index e2718a484..fddcc7b77 100644 --- a/lib/concurrent-ruby/concurrent/atomic/reentrant_read_write_lock.rb +++ b/lib/concurrent-ruby/concurrent/atomic/reentrant_read_write_lock.rb @@ -241,7 +241,10 @@ def try_read_lock # # @return [Boolean] true if the lock is successfully released def release_read_lock - held = @HeldCount.value = @HeldCount.value - 1 + held = @HeldCount.value + raise IllegalOperationError, "Cannot release a read lock which is not held" if held & READ_LOCK_MASK == 0 + + held = @HeldCount.value = held - 1 rlocks_held = held & READ_LOCK_MASK if rlocks_held == 0 c = @Counter.update { |counter| counter - 1 } @@ -249,8 +252,6 @@ def release_read_lock if waiting_or_running_writer?(c) && running_readers(c) == 0 @WriteQueue.signal end - elsif rlocks_held == READ_LOCK_MASK - raise IllegalOperationError, "Cannot release a read lock which is not held" end true end @@ -334,14 +335,15 @@ def try_write_lock # # @return [Boolean] true if the lock is successfully released def release_write_lock - held = @HeldCount.value = @HeldCount.value - WRITE_LOCK_HELD + held = @HeldCount.value + raise IllegalOperationError, "Cannot release a write lock which is not held" if held & WRITE_LOCK_MASK == 0 + + held = @HeldCount.value = held - WRITE_LOCK_HELD wlocks_held = held & WRITE_LOCK_MASK if wlocks_held == 0 c = @Counter.update { |counter| counter - RUNNING_WRITER } @ReadQueue.broadcast @WriteQueue.signal if waiting_writers(c) > 0 - elsif wlocks_held == WRITE_LOCK_MASK - raise IllegalOperationError, "Cannot release a write lock which is not held" end true end diff --git a/spec/concurrent/atomic/read_write_lock_spec.rb b/spec/concurrent/atomic/read_write_lock_spec.rb index 52e4d7273..327cd2a08 100644 --- a/spec/concurrent/atomic/read_write_lock_spec.rb +++ b/spec/concurrent/atomic/read_write_lock_spec.rb @@ -338,6 +338,13 @@ module Concurrent it 'raises an exception if the lock was never set' do expect { subject.release_read_lock }.to raise_error(IllegalOperationError) end + + it 'does not corrupt the lock state after an invalid release' do + expect { subject.release_read_lock }.to raise_error(IllegalOperationError) + + expect(subject.acquire_read_lock).to be true + expect(subject.release_read_lock).to be true + end end context '#acquire_write_lock' do @@ -494,6 +501,13 @@ module Concurrent expect { subject.release_write_lock }.to raise_error(IllegalOperationError) end + it 'does not corrupt the lock state after an invalid release' do + expect { subject.release_write_lock }.to raise_error(IllegalOperationError) + + expect(subject.acquire_write_lock).to be true + expect(subject.release_write_lock).to be true + end + it 'raises an exception if called by a thread that did not acquire the write lock' do subject.acquire_write_lock Thread.new {