Skip to content
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

Prevent race condition when using requests_futures and ThreadPoolExecutor #183

Merged
merged 3 commits into from
Apr 19, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 33 additions & 25 deletions requests_mock/mocker.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
# under the License.

import functools
import threading
import types

import requests
Expand All @@ -29,6 +30,8 @@

_original_send = requests.Session.send

_send_lock = threading.Lock()


def _is_bound_method(method):
"""
Expand Down Expand Up @@ -127,31 +130,36 @@ def _fake_get_adapter(session, url):
return self._adapter

def _fake_send(session, request, **kwargs):
# mock get_adapter
_set_method(session, "get_adapter", _fake_get_adapter)

# NOTE(jamielennox): self._last_send vs _original_send. Whilst it
# seems like here we would use _last_send there is the possibility
# that the user has messed up and is somehow nesting their mockers.
# If we call last_send at this point then we end up calling this
# function again and the outer level adapter ends up winning.
# All we really care about here is that our adapter is in place
# before calling send so we always jump directly to the real
# function so that our most recently patched send call ends up
# putting in the most recent adapter. It feels funny, but it works.

try:
return _original_send(session, request, **kwargs)
except exceptions.NoMockAddress:
if not self.real_http:
raise
except adapter._RunRealHTTP:
# this mocker wants you to run the request through the real
# requests library rather than the mocking. Let it.
pass
finally:
# restore get_adapter
_set_method(session, "get_adapter", self._last_get_adapter)
# NOTE(phodge): we need to use a threading lock here in case there
# are multiple threads running - one thread could restore the
# original get_adapter() just as a second thread is about to
# execute _original_send() below
with _send_lock:
# mock get_adapter
_set_method(session, "get_adapter", _fake_get_adapter)

# NOTE(jamielennox): self._last_send vs _original_send. Whilst it
# seems like here we would use _last_send there is the possibility
# that the user has messed up and is somehow nesting their mockers.
# If we call last_send at this point then we end up calling this
# function again and the outer level adapter ends up winning.
# All we really care about here is that our adapter is in place
# before calling send so we always jump directly to the real
# function so that our most recently patched send call ends up
# putting in the most recent adapter. It feels funny, but it works.

try:
return _original_send(session, request, **kwargs)
except exceptions.NoMockAddress:
if not self.real_http:
raise
except adapter._RunRealHTTP:
# this mocker wants you to run the request through the real
# requests library rather than the mocking. Let it.
pass
finally:
# restore get_adapter
_set_method(session, "get_adapter", self._last_get_adapter)

# if we are here it means we must run the real http request
# Or, with nested mocks, to the parent mock, that is why we use
Expand Down
1 change: 1 addition & 0 deletions test-requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,4 @@ pytest
sphinx
testrepository>=0.0.18
testtools
requests_futures
34 changes: 34 additions & 0 deletions tests/pytest/test_with_pytest.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,40 @@ def test_mixed_mocks():
assert text == 'global' # nosec


def test_threaded_sessions():
"""
When using requests_futures.FuturesSession() with a ThreadPoolExecutor
there is a race condition where one threaded request removes the
monkeypatched get_adapter() method from the Session before another threaded
request is finished using it.
"""
from requests_futures.sessions import FuturesSession

url1 = 'http://www.example.com/requests-mock-fake-url1'
url2 = 'http://www.example.com/requests-mock-fake-url2'

with requests_mock.Mocker() as m:
# respond with 204 so we know its us
m.get(url1, status_code=204)
m.get(url2, status_code=204)

# NOTE(phodge): just firing off two .get() requests right after each
# other was a pretty reliable way to reproduce the race condition on my
# intel Macbook Pro but YMMV. Guaranteeing the race condition to
# reappear might require replacing the Session.send() with a wrapper
# that delays kicking off the request for url1 until the request for
# url2 has restored the original session.get_adapter(), but replacing
# Session.send() could be difficult because the requests_mock.Mocker()
# context manager has *already* monkeypatched this method.
session = FuturesSession()
future1 = session.get(url1)
future2 = session.get(url2)

# verify both requests were handled by the mock dispatcher
assert future1.result().status_code == 204
assert future2.result().status_code == 204


class TestClass(object):

def configure(self, requests_mock):
Expand Down