miyagawa / faraday-cookie_jar

Client-side cookie management for Faraday
MIT License
76 stars 12 forks source link

Not thread-safe #16

Open Roguelazer opened 1 year ago

Roguelazer commented 1 year ago

Using Faraday with this middleware from multiple threads/fibers results in (very infrequent) crashes with the following:

vendor/bundle/ruby/3.1.0/gems/http-cookie-1.0.5/lib/http/cookie_jar/hash_store.rb:55:in `add': can't add a new key into hash during iteration (RuntimeError)
    from vendor/bundle/ruby/3.1.0/gems/http-cookie-1.0.5/lib/http/cookie_jar.rb:106:in `add'
    from vendor/bundle/ruby/3.1.0/gems/http-cookie-1.0.5/lib/http/cookie_jar.rb:192:in `block in parse'
    from vendor/bundle/ruby/3.1.0/gems/http-cookie-1.0.5/lib/http/cookie.rb:322:in `block (2 levels) in parse'
    from vendor/bundle/ruby/3.1.0/gems/http-cookie-1.0.5/lib/http/cookie/scanner.rb:214:in `scan_set_cookie'
    from vendor/bundle/ruby/3.1.0/gems/http-cookie-1.0.5/lib/http/cookie.rb:281:in `block in parse'
    from vendor/bundle/ruby/3.1.0/gems/http-cookie-1.0.5/lib/http/cookie.rb:280:in `parse'
    from vendor/bundle/ruby/3.1.0/gems/http-cookie-1.0.5/lib/http/cookie_jar.rb:191:in `parse'
    from vendor/bundle/ruby/3.1.0/gems/faraday-cookie_jar-0.0.7/lib/faraday/cookie_jar.rb:27:in `block in call'
    from vendor/bundle/ruby/3.1.0/gems/faraday-1.10.3/lib/faraday/response.rb:61:in `on_complete'
    from vendor/bundle/ruby/3.1.0/gems/faraday-cookie_jar-0.0.7/lib/faraday/cookie_jar.rb:24:in `call'

What do you think about wrapping @jar in a mutex?

miyagawa commented 1 year ago

I feel like this should be addressed in http-cookie gem. The gem has Mutex added in many places so I'm surprised that this is happening.