-
Notifications
You must be signed in to change notification settings - Fork 565
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Change httpclient adapter to use thread local variables for thread safety #300
Conversation
Cheers. One question: Is there a reason for using non http client specific global variable |
Since that instance variable is shared across threads it's not safe to have simultaneous writes to an Array or Hash. Adding to the hash is not an atomic operation so you could trash the hash even when using different keys. Make sense? |
I see. Ok that makes sense, cheers. It would be nice to make it at least in part http client specific. Thread.current[:webmock_responses] is very global. It could be as well used by other adapters. |
ok, updated... |
Change httpclient adapter to use thread local variables for thread safety
I think I have merged it too early. It seems to break http client support in ruby 1.8.7 |
Ruby 1.8.7, what's that :-) seriously isn't that deprecated? I guess if you really want to still support 1.8.7 you can back out my changes, then when I get a chance I can look into it. |
whether I like it or not 1.8.7 is still supported by WebMock 1.x :) |
I reverted it. It breaks specs in async mode on Ruby 1.8.7. No idea why yet. |
…ead_safe"" This reverts commit 20d49b0.
@tbeauvais Just FYI I have re-introduced your excellent solution to the thread safety problem of the httpclient adapter. Thank you! Ruby 1.8.7 is not an issue anymore ;) |
This fixes a threading issue when doing load testing using webmock for our HttpClient service calls using JRuby. The httpclient_adapter was using shared instance variables which caused response corruption.
Oh I noticed before I even started that there were several spec failures. I looked it to it quickly and noticed that calls http://www.example.com/ now results in 200, with actual content. Is someone looking at that?