`Concurrent::Hash` default initialization is not fully thread-safe

[mensfeld]

Based on the docs:

A thread-safe subclass of Hash. This version locks against the object itself for every method call, ensuring only one thread can be reading or writing at a time. This includes iteration methods like #each, which takes the lock repeatedly when reading an item.

Given this code:

h = Concurrent::Hash.new do |hash, key|
  hash[key] = Concurrent::Array.new
end

the initialization is not thread-safe.

Note from @eregon, the thread-safe variant of this code is:

h = Concurrent::Map.new do |hash, key|
  hash.compute_if_absent(key) { Concurrent::Array.new }
end

Obviously the latter part of the doc indicates that:

ensuring only one thread can be reading or writing at a time

but the initial part makes it confusing:

This version locks against the object itself for every method call

It can be demoed by running this code:

require 'concurrent-ruby'

1000.times do
  h = Concurrent::Hash.new do |hash, key|
    hash[key] = Concurrent::Array.new
  end

  100.times.map do
    Thread.new do
      h[:na] << true
    end
  end.each(&:join)

  raise if h[:na].count != 100
end

I would expect to either:

1. Have the initialization block behind a mutex - so there is no conflict
2. Have the docs updated (I can do that)

Works like so:

require 'concurrent-ruby'

m = Mutex.new

1000.times do
  h = Concurrent::Hash.new do |hash, key|
    m.synchronize do
      break hash[key] if hash.key?(key)

      hash[key] = Concurrent::Array.new
    end
  end

  100.times.map do
    Thread.new do
      h[:na] << true
    end
  end.each(&:join)

  raise if h[:na].count != 100
end

[nijikon]

Oh, wow. This is interesting.

[mensfeld]

Same applies to the Concurrent::Map:

require 'concurrent-ruby'

1000.times do
  h = Concurrent::Map.new do |hash, key|
    hash[key] = Concurrent::Array.new
  end

  100.times.map do
    Thread.new do
      h[:na] << true
    end
  end.each(&:join)

  raise if h[:na].count != 100
end