Skip to content

Commit 5f0db22

Browse files
committed
Lock#wait releases lock when going to sleep, even if called with lock held
Rubinius.lock counts how many times an object has been locked, and only releases the lock when it has been unlocked an equal number of times. Therefore, if Lock#wait is called on a Lock which was *already* locked, the call to Rubinius.unlock which occurs before sleeping does not actually release the lock. This makes it impossible for other threads to wake it up with #signal or #broadcast. A solution is to make sure that #synchronize never calls Rubinius.lock on an object which is already locked.
1 parent 1b69f76 commit 5f0db22

File tree

2 files changed

+37
-1
lines changed

2 files changed

+37
-1
lines changed

lib/concurrent/synchronization/rbx_object.rb

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,26 @@ module Synchronization
88
class RbxObject < AbstractObject
99
def initialize
1010
@__Waiters__ = []
11+
@__owner__ = nil
1112
ensure_ivar_visibility!
1213
end
1314

1415
protected
1516

1617
def synchronize(&block)
17-
Rubinius.synchronize(self, &block)
18+
if @__owner__ == Thread.current
19+
yield
20+
else
21+
Rubinius.lock(self)
22+
begin
23+
@__owner__ = Thread.current
24+
result = yield
25+
ensure
26+
@__owner__ = nil
27+
Rubinius.unlock(self)
28+
result
29+
end
30+
end
1831
end
1932

2033
def ns_wait(timeout = nil)

spec/concurrent/synchronization_spec.rb

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
require 'timeout'
2+
13
module Concurrent
24

35
describe Synchronization do
@@ -26,6 +28,8 @@ def wait(timeout = nil)
2628
synchronize { ns_wait(timeout) }
2729
end
2830

31+
public :synchronize
32+
2933
private
3034

3135
def ns_initialize
@@ -57,6 +61,25 @@ def ns_initialize
5761
expect(t.status).to eq false
5862
expect(t.alive?).to eq false
5963
end
64+
65+
it 'releases the lock on the current object' do
66+
expect { Timeout.timeout(3) do
67+
t = Thread.new { subject.wait }
68+
sleep 0.1
69+
expect(t.status).to eq 'sleep'
70+
subject.synchronize {} # we will deadlock here if #wait doesn't release lock
71+
end }.not_to raise_error
72+
end
73+
74+
it 'can be called from within a #synchronize block' do
75+
expect { Timeout.timeout(3) do
76+
# #wait should release lock, even if it was already held on entry
77+
t = Thread.new { subject.synchronize { subject.wait }}
78+
sleep 0.1
79+
expect(t.status).to eq 'sleep'
80+
subject.synchronize {} # we will deadlock here if lock wasn't released
81+
end }.not_to raise_error
82+
end
6083
end
6184

6285
describe '#synchronize' do

0 commit comments

Comments
 (0)