From cf3d1cfd92e6824ed16cecb27f77067593d8f4e9 Mon Sep 17 00:00:00 2001 From: Adam Harwood Date: Fri, 16 Oct 2020 13:48:31 +1000 Subject: [PATCH] Make httpclient_adapter threadsafe httpclient_adapter uses some standard Ruby hashes, which produce unexpected results when working in a true multi-threaded (e.g. JRuby) environment. Multiple threads doing puts and deletes on these hashes result in nils sometimes being returned for a properly stubbed request. These errors would manifest as: webmock-3.9.2/lib/webmock/http_lib_adapters/httpclient_adapter.rb:113:in `build_httpclient_response': undefined method `body' for nil:NilClass (NoMethodError) Unfortunately these errors are extremly difficult to replicate. This PR is a follow-on from https://github.com/bblimke/webmock/pull/908, and represents the last thread-safety issue we've been able to find. --- .../http_lib_adapters/httpclient_adapter.rb | 29 +++++++++++++++---- 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/lib/webmock/http_lib_adapters/httpclient_adapter.rb b/lib/webmock/http_lib_adapters/httpclient_adapter.rb index 9fc4e123b..bacbf5dad 100644 --- a/lib/webmock/http_lib_adapters/httpclient_adapter.rb +++ b/lib/webmock/http_lib_adapters/httpclient_adapter.rb @@ -43,6 +43,9 @@ def self.disable! end module WebMockHTTPClients + + REQUEST_RESPONSE_LOCK = Mutex.new + def do_get_block(req, proxy, conn, &block) do_get(req, proxy, conn, false, &block) end @@ -57,7 +60,7 @@ def do_get(req, proxy, conn, stream = false, &block) WebMock::RequestRegistry.instance.requested_signatures.put(request_signature) if webmock_responses[request_signature] - webmock_response = webmock_responses.delete(request_signature) + webmock_response = synchronize_request_response { webmock_responses.delete(request_signature) } response = build_httpclient_response(webmock_response, stream, req.header, &block) @request_filter.each do |filter| filter.filter_response(req, response) @@ -68,7 +71,7 @@ def do_get(req, proxy, conn, stream = false, &block) res elsif WebMock.net_connect_allowed?(request_signature.uri) # in case there is a nil entry in the hash... - webmock_responses.delete(request_signature) + synchronize_request_response { webmock_responses.delete(request_signature) } res = if stream do_get_stream_without_webmock(req, proxy, conn, &block) @@ -100,7 +103,7 @@ def do_get(req, proxy, conn, stream = false, &block) def do_request_async(method, uri, query, body, extheader) req = create_request(method, uri, query, body, extheader) request_signature = build_request_signature(req) - webmock_request_signatures << request_signature + synchronize_request_response { webmock_request_signatures << request_signature } if webmock_responses[request_signature] || WebMock.net_connect_allowed?(request_signature.uri) super @@ -184,7 +187,9 @@ def build_request_signature(req, reuse_existing = false) def webmock_responses @webmock_responses ||= Hash.new do |hash, request_signature| - hash[request_signature] = WebMock::StubRegistry.instance.response_for_request(request_signature) + synchronize_request_response do + hash[request_signature] = WebMock::StubRegistry.instance.response_for_request(request_signature) + end end end @@ -193,8 +198,10 @@ def webmock_request_signatures end def previous_signature_for(signature) - return nil unless index = webmock_request_signatures.index(signature) - webmock_request_signatures.delete_at(index) + synchronize_request_response do + return nil unless index = webmock_request_signatures.index(signature) + webmock_request_signatures.delete_at(index) + end end private @@ -209,6 +216,16 @@ def headers_from_session(uri) hdrs end end + + def synchronize_request_response + if REQUEST_RESPONSE_LOCK.owned? + yield + else + REQUEST_RESPONSE_LOCK.synchronize do + yield + end + end + end end class WebMockHTTPClient < HTTPClient