Skip to content

Commit

Permalink
get-or-create-connection should be an atomic operation (#81)
Browse files Browse the repository at this point in the history
  • Loading branch information
tomchristie committed May 11, 2020
1 parent 2bde5f6 commit c634ba7
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 16 deletions.
26 changes: 18 additions & 8 deletions httpcore/_async/connection_pool.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
from ssl import SSLContext
from typing import AsyncIterator, Callable, Dict, Optional, Set, Tuple

from .._backends.auto import AsyncSemaphore, AutoBackend
from .._backends.auto import AsyncLock, AsyncSemaphore, AutoBackend
from .._exceptions import PoolTimeout
from .._threadlock import ThreadLock
from .._types import URL, Headers, Origin, TimeoutDict
Expand Down Expand Up @@ -107,6 +107,12 @@ def _connection_semaphore(self) -> AsyncSemaphore:

return self._internal_semaphore

@property
def _connection_acquiry_lock(self) -> AsyncLock:
if not hasattr(self, "_internal_connection_acquiry_lock"):
self._internal_connection_acquiry_lock = self._backend.create_lock()
return self._internal_connection_acquiry_lock

async def request(
self,
method: bytes,
Expand All @@ -123,13 +129,17 @@ async def request(

connection: Optional[AsyncHTTPConnection] = None
while connection is None:
connection = await self._get_connection_from_pool(origin)

if connection is None:
connection = AsyncHTTPConnection(
origin=origin, http2=self._http2, ssl_context=self._ssl_context,
)
await self._add_to_pool(connection, timeout=timeout)
async with self._connection_acquiry_lock:
# We get-or-create a connection as an atomic operation, to ensure
# that HTTP/2 requests issued in close concurrency will end up
# on the same connection.
connection = await self._get_connection_from_pool(origin)

if connection is None:
connection = AsyncHTTPConnection(
origin=origin, http2=self._http2, ssl_context=self._ssl_context,
)
await self._add_to_pool(connection, timeout=timeout)

try:
response = await connection.request(
Expand Down
26 changes: 18 additions & 8 deletions httpcore/_sync/connection_pool.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
from ssl import SSLContext
from typing import Iterator, Callable, Dict, Optional, Set, Tuple

from .._backends.auto import SyncSemaphore, SyncBackend
from .._backends.auto import SyncLock, SyncSemaphore, SyncBackend
from .._exceptions import PoolTimeout
from .._threadlock import ThreadLock
from .._types import URL, Headers, Origin, TimeoutDict
Expand Down Expand Up @@ -107,6 +107,12 @@ def _connection_semaphore(self) -> SyncSemaphore:

return self._internal_semaphore

@property
def _connection_acquiry_lock(self) -> SyncLock:
if not hasattr(self, "_internal_connection_acquiry_lock"):
self._internal_connection_acquiry_lock = self._backend.create_lock()
return self._internal_connection_acquiry_lock

def request(
self,
method: bytes,
Expand All @@ -123,13 +129,17 @@ def request(

connection: Optional[SyncHTTPConnection] = None
while connection is None:
connection = self._get_connection_from_pool(origin)

if connection is None:
connection = SyncHTTPConnection(
origin=origin, http2=self._http2, ssl_context=self._ssl_context,
)
self._add_to_pool(connection, timeout=timeout)
with self._connection_acquiry_lock:
# We get-or-create a connection as an atomic operation, to ensure
# that HTTP/2 requests issued in close concurrency will end up
# on the same connection.
connection = self._get_connection_from_pool(origin)

if connection is None:
connection = SyncHTTPConnection(
origin=origin, http2=self._http2, ssl_context=self._ssl_context,
)
self._add_to_pool(connection, timeout=timeout)

try:
response = connection.request(
Expand Down

0 comments on commit c634ba7

Please sign in to comment.