Skip to content

Commit

Permalink
Make httpclient_adapter threadsafe
Browse files Browse the repository at this point in the history
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 bblimke#908, and represents the last thread-safety issue we've been able to find.
  • Loading branch information
Adam Harwood committed Oct 16, 2020
1 parent 48cc7c5 commit cf3d1cf
Showing 1 changed file with 23 additions and 6 deletions.
29 changes: 23 additions & 6 deletions lib/webmock/http_lib_adapters/httpclient_adapter.rb
Expand Up @@ -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
Expand All @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand All @@ -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
Expand All @@ -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
Expand Down

0 comments on commit cf3d1cf

Please sign in to comment.