Unsafe concurrent Hash access

[eregon]

It happens for SEQUENCE_UPCASED_PERCENT_ENCODING_TABLE and SEQUENCE_ENCODING_TABLE
See oracle/truffleruby#3166 (comment)

This happens on TruffleRuby and possibly on JRuby.
On CRuby the error will not happen due to the GIL.
On CRuby it may still cause duplicate computation and assignment for the same key.

A good solution would be to use a Concurrent::Map from concurrent-ruby instead of raw Hash there.
And the same for SEQUENCE_ENCODING_TABLE just above.
(as a note, this is the correct pattern: ruby-concurrency/concurrent-ruby#970)
Then it's fast on CRuby and is thread-safe and scales on no-GVL Rubies.

Of course this would add a dependency on concurrent-ruby for this gem.

[eregon]

An alternative would be to use a Mutex around all accesses to these global Hash'es.
But that would be slower on CRuby due to the extra Mutex on read, and not scale on no-GVL Rubies.
So performance-wise it's strictly worse than concurrent-ruby for all Rubies.

[dentarg]

It happens for SEQUENCE_UPCASED_PERCENT_ENCODING_TABLE and SEQUENCE_ENCODING_TABLE

Because I looked it up I'll note that those were introduced with 880b34c

[dentarg]

A good solution would be to use a Concurrent::Map from concurrent-ruby instead of raw Hash there

Would that help even before ruby-concurrency/concurrent-ruby#970 is addressed?

[eregon]

Yes, the only thing I meant by that link is one should use that compute_if_absent pattern to avoid assigning a given key twice.

[dentarg]

Right, so the way to address this (with concurrent-ruby) is using Concurrent::Map and compute_if_absent like this?

[eregon]

Yes

[eregon]

More comments by @byroot at oracle/truffleruby#3166 (comment)

[eregon]

@byroot Any chance you could take a look at this? :)

It'd be good to figure out what's the desired solution for addressable.

[byroot]

I was quite busy lately. I'll try to find some time to dig into this next week.