Ensure HTTP/2 connections are gracefully closed on async cancellation

[karpetrosyan]

Closes #756

Checklist

• I understand that this PR may be closed in case there was no previous discussion. (This doesn't apply to typos!)
• I've added a test for each change that was introduced, and I tried as much as possible to make a single atomic change.
• I've updated the documentation accordingly.

TODO

• Add failing tests
• Gracefully handle cancellation for unstarted http/2 requests
• Update changelog

[T-256]

could (async )with self._state_lock: move under _state_housekeeping function?

[zanieb]

could (async )with self._state_lock: move under _state_housekeeping function?

I agree, it's always used.

[lovelydinosaur]

At thanks! The failing test case is especially helpful here.

Pretty sure this resolution is overcomplicated, tho. I think we actually want here is?...

1. This existing try...except needs to be expanded...

httpcore/httpcore/_async/http2.py

Lines 106 to 113 in 21e47a7

	try:
	kwargs = {"request": request}
	async with Trace("send_connection_init", logger, request, kwargs):
	await self._send_connection_init(**kwargs)
	except BaseException as exc:
	with AsyncShieldCancellation():
	await self.aclose()
	raise exc

...so that it instead covers all of this block...

httpcore/httpcore/_async/http2.py

Lines 106 to 127 in 21e47a7

	try:
	kwargs = {"request": request}
	async with Trace("send_connection_init", logger, request, kwargs):
	await self._send_connection_init(**kwargs)
	except BaseException as exc:
	with AsyncShieldCancellation():
	await self.aclose()
	raise exc
	self._sent_connection_init = True
	# Initially start with just 1 until the remote server provides
	# its max_concurrent_streams value
	self._max_streams = 1
	local_settings_max_streams = (
	self._h2_state.local_settings.max_concurrent_streams
	)
	self._max_streams_semaphore = AsyncSemaphore(local_settings_max_streams)
	for _ in range(local_settings_max_streams - self._max_streams):
	await self._max_streams_semaphore.acquire()

ie. the except case to move down to line 127 there.

2. This call to acquire()...

httpcore/httpcore/_async/http2.py

Line 129 in 21e47a7

await self._max_streams_semaphore.acquire()

Should also be surrounded with...

try:
    await self._max_streams_semaphore.acquire()
 except BaseException as exc: 
     with AsyncShieldCancellation(): 
         await self.aclose() 
     raise exc

We could actually avoid (1) if we instead addressed the refactoring suggested in #738 or refactored self._send_connection_init(**kwargs) so that our initial handshake waits for the settings response before we instantiate our max_streams_semaphore. But let's not be getting into that just now.

[lovelydinosaur]

Ugh. I need to think about this one more clearly, it's making my brain mush.

[karpetrosyan]

I would recommend using this stupid solution for the time being and not waiting for http/2 refactoring from #770

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.