Race in connection pool

[nelhage]

If I make a number of GET requests in concurrent threads using a shared httpcore.ConnectionPool (in my case, via httpx), I reliably hit this assertion failure.

Test script:

import httpx
from concurrent.futures import ThreadPoolExecutor

http_client = httpx.Client()

def get_one(_):
 http_client.get("https://example.com/")


def main():
 n_threads = 4
 n_ops = 2000

 with ThreadPoolExecutor(n_threads) as tpe:
 list(tpe.map(get_one, range(n_ops)))


if __name__ == "__main__":
 main()

Output:

❯ python httpx_race.py
Traceback (most recent call last):
 File "/Users/nelhage/code/sandbox/httpx_race.py", line 20, in <module>
 main()
 File "/Users/nelhage/code/sandbox/httpx_race.py", line 16, in main
 list(tpe.map(get_one, range(n_ops)))
 File "/Users/nelhage/miniforge3/envs/py310/lib/python3.10/concurrent/futures/_base.py", line 621, in result_iterator
 yield _result_or_cancel(fs.pop())
 File "/Users/nelhage/miniforge3/envs/py310/lib/python3.10/concurrent/futures/_base.py", line 319, in _result_or_cancel
 return fut.result(timeout)
 File "/Users/nelhage/miniforge3/envs/py310/lib/python3.10/concurrent/futures/_base.py", line 451, in result
 return self.__get_result()
 File "/Users/nelhage/miniforge3/envs/py310/lib/python3.10/concurrent/futures/_base.py", line 403, in __get_result
 raise self._exception
 File "/Users/nelhage/miniforge3/envs/py310/lib/python3.10/concurrent/futures/thread.py", line 58, in run
 result = self.fn(*self.args, **self.kwargs)
 File "/Users/nelhage/code/sandbox/httpx_race.py", line 8, in get_one
 http_client.get("https://example.com/")
 File "/Users/nelhage/miniforge3/envs/py310/lib/python3.10/site-packages/httpx/_client.py", line 1039, in get
 return self.request(
 File "/Users/nelhage/miniforge3/envs/py310/lib/python3.10/site-packages/httpx/_client.py", line 815, in request
 return self.send(request, auth=auth, follow_redirects=follow_redirects)
 File "/Users/nelhage/miniforge3/envs/py310/lib/python3.10/site-packages/httpx/_client.py", line 902, in send
 response = self._send_handling_auth(
 File "/Users/nelhage/miniforge3/envs/py310/lib/python3.10/site-packages/httpx/_client.py", line 930, in _send_handling_auth
 response = self._send_handling_redirects(
 File "/Users/nelhage/miniforge3/envs/py310/lib/python3.10/site-packages/httpx/_client.py", line 967, in _send_handling_redirects
 response = self._send_single_request(request)
 File "/Users/nelhage/miniforge3/envs/py310/lib/python3.10/site-packages/httpx/_client.py", line 1003, in _send_single_request
 response = transport.handle_request(request)
 File "/Users/nelhage/miniforge3/envs/py310/lib/python3.10/site-packages/httpx/_transports/default.py", line 218, in handle_request
 resp = self._pool.handle_request(req)
 File "/Users/nelhage/miniforge3/envs/py310/lib/python3.10/site-packages/httpcore/_sync/connection_pool.py", line 221, in handle_request
 self._attempt_to_acquire_connection(status)
 File "/Users/nelhage/miniforge3/envs/py310/lib/python3.10/site-packages/httpcore/_sync/connection_pool.py", line 160, in _attempt_to_acquire_connection
 status.set_connection(connection)
 File "/Users/nelhage/miniforge3/envs/py310/lib/python3.10/site-packages/httpcore/_sync/connection_pool.py", line 22, in set_connection
 assert self.connection is None
AssertionError

The proximate issue appears to be in these lines:

httpcore/httpcore/_sync/connection_pool.py

Lines 216 to 221 in 2f1cf70

	status = RequestStatus(request)
	self._requests.append(status)
	with self._pool_lock:
	self._close_expired_connections()
	self._attempt_to_acquire_connection(status)

As soon as we add status to self._requests outside of the lock (line 217), it becomes visible to other threads. One of those threads may complete a request, and may assign us a connection:

httpcore/httpcore/_sync/connection_pool.py

Lines 287 to 289 in 2f1cf70

	for status in self._requests:
	if status.connection is None:
	acquired = self._attempt_to_acquire_connection(status)

We will then acquire the lock and enter _attempt_to_acquire_connection , which will also attempt to assign a connection.

An immediate fix would be to move line 217 inside of the lock, but _requests is accessed outside of the lock in various other places so I'm not sure that this would be sufficient or the best fix.

[Zac-HD]

Note that the proximate cause noted above is a sufficient but not necessary condition - we first encountered thread-safety problems in version 0.15, before #580 moved self._requests.append(status) outside the lock.