Skip to content

Initialize the monitor for new subarrays on Rubinius #665

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 23, 2017
Merged

Initialize the monitor for new subarrays on Rubinius #665

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 16 additions & 5 deletions lib/concurrent/thread_safe/util/array_hash_rbx.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,22 @@ def self.allocate
end

klass.superclass.instance_methods(false).each do |method|
klass.class_eval <<-RUBY, __FILE__, __LINE__ + 1
def #{method}(*args)
@_monitor.synchronize { super }
end
RUBY
case method
when :new_range, :new_reserved
klass.class_eval <<-RUBY, __FILE__, __LINE__ + 1
def #{method}(*args)
obj = super
obj.send(:_mon_initialize)
obj
end
RUBY
else
klass.class_eval <<-RUBY, __FILE__, __LINE__ + 1
def #{method}(*args)
@_monitor.synchronize { super }
end
RUBY
end
end
end
end
Expand Down
10 changes: 10 additions & 0 deletions spec/concurrent/array_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,5 +14,15 @@ module Concurrent
end
end.map(&:join)
end

describe '#slice' do
# This is mostly relevant on Rubinius and Truffle
Copy link
Collaborator

@eregon eregon Jul 15, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW TruffleRuby tries to avoid extra public methods, so new_range/new_reserved are not defined on Array in TruffleRuby.
Arguably, this is a Rubinius bug to expose internal methods as public.

Copy link
Contributor Author

@meineerde meineerde Jul 15, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem is not so much that this method is public rather than that they are "special" to Rubinius in that they create objects without calling allocate. They would still have to be overwritten by concurrent-ruby if they were private, although it would have to be done slightly different...

As for the comment itself, I have not tested this on Truffle nor have I specifically checked their implementation to be honest. The comment might thus be not 100% exact in this regard. However, since the monitor is only used on Rubinius and Truffle at all, the test still applies to this locking variant used here.

it 'correctly initializes the monitor' do
ary.concat([0, 1, 2, 3, 4, 5, 6, 7, 8])

sliced = ary.slice!(0..2)
expect { sliced[0] }.not_to raise_error
end
end
end
end