search

dry-rb / dry-schema

Coercion and validation for data structures
https://dry-rb.org/gems/dry-schema
MIT License
421 stars 110 forks source link

Make the yaml cache fully thread-safe #440

Closed mensfeld closed 1 year ago

mensfeld commented 1 year ago 
Concurrent::Map.new { |h, k| h[k] = Concurrent::Map.new }

is prone to a race condition, thus the cache won't be fully thread safe if definitions are parsed in multiple threads.

This PR fixes that.

Ref: https://github.com/ruby-concurrency/concurrent-ruby/issues/970

mensfeld commented 1 year ago

specs dont seem to be related to this change

mensfeld commented 1 year ago

Got it. WIll look at this today.

flash-gordon commented 1 year ago

I have main failing locally too, investigating

Finished in 10.7 seconds (files took 3.15 seconds to load)
3183 examples, 39 failures, 4 pending
flash-gordon commented 1 year ago

main is green, merging

granthusbands commented 1 year ago

@mensfeld You missed the ||= that you've fixed in pull requests for other projects.

mensfeld commented 1 year ago

@granthusbands I did not miss it. I explicitly do not do it here to make the change small. I pointed out in the article that I wanted to keep the scope small because I do not know how given libs operate.

that you've fixed in pull requests for other projects.

Not exactly the same. In most cases, I fixed the internal ||=, not the external. Nonetheless you are right and feel free to create a PR :) @solnic should be happy