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

Threading.Lock causes nested test to hang #201

Closed
jamielennox opened this issue Apr 21, 2022 · 2 comments
Closed

Threading.Lock causes nested test to hang #201

jamielennox opened this issue Apr 21, 2022 · 2 comments
Milestone

Comments

@jamielennox
Copy link
Owner

This is currently breaking github tests and is reproducable on my local. I don't know how it got merged without triggering the test. I'm going to revert the commits for now, they're pretty small and can go back in later.

As part of #183 we added a threading.Lock to the core send() function. This unfortunately appears to break if you have nested mockers, which i don't like but know that people do. Specifically this test:

def test_redirect_and_nesting():
url_inner = "inner-mock://example.test/"
url_middle = "middle-mock://example.test/"
url_outer = "outer-mock://example.test/"
url_base = "https://www.example.com/"
text_middle = 'middle' + url_middle
text_outer = 'outer' + url_outer
text_base = 'outer' + url_base
with requests_mock.Mocker() as outer_mock:
outer_mock.get(url_base, text=text_base)
outer_mock.get(url_outer, text=text_outer)
with requests_mock.Mocker(real_http=True) as middle_mock:
middle_mock.get(url_middle, text=text_middle)
with requests_mock.Mocker() as inner_mock:
inner_mock.post(url_inner,
status_code=HTTP_STATUS_FOUND,
headers={'location': url_base})
inner_mock.get(url_base, real_http=True)
assert text_base == requests.post(url_inner).text # nosec
with pytest.raises(requests_mock.NoMockAddress):
requests.get(url_middle)
with pytest.raises(requests_mock.NoMockAddress):
requests.get(url_outer)
# back to middle mock
with pytest.raises(requests_mock.NoMockAddress):
requests.post(url_inner)
assert text_middle == requests.get(url_middle).text # nosec
assert text_outer == requests.get(url_outer).text # nosec
# back to outter mock
with pytest.raises(requests_mock.NoMockAddress):
requests.post(url_inner)
with pytest.raises(requests_mock.NoMockAddress):
requests.get(url_middle)
assert text_outer == requests.get(url_outer).text # nosec

An easy solution I found to the problem is to use an RLock instead, but it does raise the question are there other places this could be a problem, and maybe is there a better way to reliably monkeypatch the function without needing to Lock?

@phodge - thoughts?

@phodge
Copy link
Contributor

phodge commented Apr 21, 2022

Hi @jamielennox: Thanks for bringing this to my attention. RLock sounds like it might work, but I can't remember the code well enough to say whether it might also lead to unexpected deadlocks. I'm keen to have the threading fixes added to the next release so I will have a look at this first thing tomorrow and see if I can come up with another fix if you haven't figured something out by then. (I'm on UTC+10 so it's too late for me to look at this tonight.)

@jamielennox jamielennox added this to the 1.10 milestone Aug 30, 2022
@jamielennox
Copy link
Owner Author

Released in 1.10.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants