Close a potentially unclosed body file before it can interfere with writing a new one

[A5rocks]

This is based on debugging in python-trio/trio#3127 and some comments from @graingert, so see that PR for specific information on what was failing (I minimized a failing example) and testing this fix for it (latest commit). Just look at the relevant github actions logs.

---

This problem only manifests on the combination of PyPy and Windows. Here's how:

• CacheControlAdapter#send calls conditional_headers
• conditional_headers calls _load_from_cache
• _load_from_cache calls body_file = self.cache.get_body(cache_url) and saves it to the response
• get_body returns an opened file
• conditional_headers uses the response (which holds the body file) to get headers
• conditional_headers returns, but doesn't explicitly finish the request
  • on cpython this is fine -- refcounting means the file gets closed
  • on pypy, this keeps the file open for a while longer
• CacheControlAdapter#send now runs super().send(request, stream, timeout, verify, cert, proxies)
  • this makes the request (good)
  • and saves the body of the new response. note that the body file may still be open on pypy (bad)
    • on unix it is OK that the bodyfile is open and we are trying to os.replace it
    • but on windows it is not OK

Thus an annoying caching bug happens. Hopefully I've described it well enough.

---

Note that this might only happen with pip's SafeFileCache (which is a subclass of SeparateBodyBaseCache). Some of the details rely on os.replace etc which doesn't happen here. But the fix applies here so...

[graingert]

I'm scratching my head why unraisablehook isn't detecting this missing with. Perhaps requests is closing the connection on GC without issuing a ResourceWarning?

[A5rocks]

It's also possible a SeparateBodyCache isn't used in the tests (I haven't checked)?

[A5rocks]

I think test_body_actually_stored_separately doesn't test the case where theres an outdated entry in cache, which is when a ResourceWarning should appear.

Otherwise, conditional_headers either:

• doesn't have a resp (so no body file to warn)
• gets shortcircuited around. see

  cachecontrol/cachecontrol/adapter.py

  Lines 70 to 74 in 34564d8

  	if cached_response:
  	return self.build_response(request, cached_response, from_cache=True)
  	# check for etags and add headers if appropriate
  	request.headers.update(self.controller.conditional_headers(request))

though I'm not completely sure, cause test_update_cached_response_with_valid_headers_separate_body looks like it tests that case...