Skip to content

Commit 4a00ff5

Browse files
authored
[5.28] Fix race condition in bolt connection pool (neo4j#1348)
The pool tries to acquire an idle connection first. If no idle connection is available, it puts itself into a queue waiting for another thread/task to return a connection before trying again. However, there is no protection against a connection being returned between the attempt to acquire an idle connection and the enqueuing. Therefore, it could happen that a thread waits indefinitely like this: Assume max pool size of 1 * T1 creates & acquires con1 * T2 checks for idle connections => None * T1 releases con1 * T2 acquires lock to be able to wait for the release event * T1 terminates * T2 times out waiting for Godot
1 parent 22838ef commit 4a00ff5

3 files changed

Lines changed: 34 additions & 8 deletions

File tree

‎src/neo4j/_async/io/_pool.py‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -394,6 +394,13 @@ async def health_check(connection_, deadline_):
394394
return connection
395395
# all connections in pool are in-use
396396
with self.lock:
397+
if any(not con.in_use for con in self.connections[address]):
398+
# between trying to acquire an idle connection and
399+
# acquiring the lock to wait for space in the pool, a
400+
# connection was released back into the pool.
401+
# => Try to acquire an idle connection again instead of
402+
# waiting.
403+
continue
397404
connection_creator = self._acquire_new_later(
398405
address,
399406
auth,

‎src/neo4j/_sync/io/_pool.py‎

Lines changed: 7 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎tests/unit/mixed/io/test_direct.py‎

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import asyncio
2121
import threading
2222
import time
23+
import traceback
2324
from asyncio import Event as AsyncEvent
2425
from threading import (
2526
Event,
@@ -68,18 +69,25 @@ def test_multithread(self, pre_populated, fake_connection_generator):
6869
connections_lock = Lock()
6970
connections = []
7071
pre_populated_connections = []
72+
thread_exceptions = []
7173

7274
def acquire_release_conn(
7375
pool_, address_, acquired_counter_, release_event_
7476
):
75-
nonlocal connections, connections_lock
76-
conn_ = pool_._acquire(address_, None, Deadline(3), None)
77-
with connections_lock:
78-
if connections is not None:
79-
connections.append(conn_)
80-
acquired_counter_.increment()
81-
release_event_.wait()
82-
pool_.release(conn_)
77+
nonlocal connections, connections_lock, thread_exceptions
78+
try:
79+
conn_ = pool_._acquire(address_, None, Deadline(3), None)
80+
try:
81+
with connections_lock:
82+
if connections is not None:
83+
connections.append(conn_)
84+
acquired_counter_.increment()
85+
release_event_.wait()
86+
finally:
87+
pool_.release(conn_)
88+
except BaseException as e:
89+
traceback.print_exception(e)
90+
thread_exceptions.append(e)
8391

8492
with FakeBoltPool(
8593
fake_connection_generator, (), max_connection_pool_size=5
@@ -126,6 +134,10 @@ def acquire_release_conn(
126134
t.join(timeout=5)
127135
if t.is_alive():
128136
raise TimeoutError(f"Joining thread timed out: {t!r}")
137+
if thread_exceptions:
138+
raise RuntimeError(
139+
"thread raised an exception"
140+
) from thread_exceptions[0]
129141
# The pool size is still 5, but all are free
130142
self.assert_pool_size(address, 0, 5, pool)
131143

0 commit comments

Comments
 (0)