should the thread_local_var_accessor gem be included within the concurrent-ruby gem?

[aks]

I've written a little gem that makes concurrent-ruby's thread-local variables easy to use, in the same manner of attr_reader, attr_writer, attr_accessor: it is called: thread_local_var_accessors

See:

• https://github.com/aks/thread_local_var_accessors
• https://rubygems.org/gems/thread_local_var_accessors

Eg:

tlv_reader :var1
tlv_writer :var2
tlv_accessor: :var3

I've been using it in my company software for some time now. It is an easy way to encourage developers to improve the thread-safetyness of their code, especially code that runs in a multi-threaded environment (eg: sidekiq tasks).

I'm happy to continue maintaining it separately, but I'm also happy to contribute it to the core concurrent-ruby gem, if that's appropriate. In the latter case, since we are already using concurrent-ruby elsewhere, for other reasons, it would mean one gem less to track for us. 😄

[eregon]

Thanks for writing up the proposal.
I don't know how often this is useful, for example if it's a ThreadLocalVar which can be stored e.g. in a constant/globally there is no need for this.

I took a quick look at the code in https://github.com/aks/thread_local_var_accessors.
One thing which does not seem thread-safe is https://github.com/aks/thread_local_var_accessors/blob/d5d4fd3418d32c1901f719da603f652b7f1cea17/lib/thread_local_var_accessors.rb#L280-L285 specifically for:

class Foo
  tlv_accessor: :tvar
  def initialize
  end

  def foo
    self.tvar = true
    p tvar
end

foo = Foo.new
8.times.map { Thread.new { foo.foo } }

Some threads might print nil instead of true because multiple threads might assign @tvar and so overwrite each other.

[eregon]

Given this seems mostly convenience on top of ThreadLocalVar to be a bit shorter (e.g. tvar/self.tvar = v vs @tvar.value/@tvar.value = v), I think it's best as its own gem.

And the fact the ThreadLocalVar is created more magically/behind-the-scenes creates the race mentioned above.
Although that could be solved e.g. by requiring to assign the ivar in initialize, and raising an exception on read/write if it's not the case. (or using a global or per-class lock but that would create lots of contention so not a good solution)

[aks]

I corrected your test example above, and ran it with thousands of threads. At no point was there ever any error.

However, I'm fine keeping the library in a separate gem.
Thanks for your reply and review.

Here's the updated test program and a sample output:

% cat tlvtest.rb
#!/usr/bin/env ruby
require 'thread_local_var_accessors'

class Foo
  include ThreadLocalVarAccessors

  tlv_accessor :tvar

  def initialize
  end

  def foo(num)
    self.tvar = num
    tvar
  end
end

obj = Foo.new

count = ARGV.shift&.to_i || 100

puts "Count: #{count}"

data_in = count.times.to_a
data_out = data_in.map { |num| Thread.new { obj.foo(num) }.run.value }

diffs = data_in - data_out
if diffs.size.zero?
  puts "no diffs"
else
  puts "Diffs: #{diffs}"
end

And, here's the output:

% $ time ./tlvtest.rb 100000
Count: 100000
no diffs
./tlvtest.rb 100000  2.11s user 1.47s system 113% cpu 3.145 total