Skip to content

Commit ce3e272

Browse files
aviadr1claude
andcommitted
test: Add failing test for set_config() blocking concurrent get()
set_config() holds the global write lock (ldclient/__init__.py) across both the construction of the replacement client and old_client.close(). ReadWriteLock holds the underlying mutex for the whole write section, so every concurrent ldclient.get() - which takes a read lock, and which sits on the hot path of every flag evaluation - blocks until both finish. The test drives a 3 second close() and measures how long a concurrent get() blocks; it currently reports 2.8s. Marked xfail strict so it fails loudly once the behaviour is fixed and the marker can be removed. This couples a network-dependent shutdown to a lock that every evaluating thread needs, so a slow or stalled close() stalls the whole application rather than just the thread that called set_config(). No fix is proposed here; this only pins the contract. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 5da1515 commit ce3e272

1 file changed

Lines changed: 55 additions & 0 deletions

File tree

ldclient/testing/test_ldclient_singleton.py

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,12 @@
11
import json
2+
import threading
3+
import time
4+
5+
import pytest
26

37
import ldclient
48
from ldclient import _reset_client
9+
from ldclient.client import LDClient
510
from ldclient.config import Config
611
from ldclient.testing.http_util import BasicResponse, start_server
712
from ldclient.testing.stub_util import make_put_event, stream_content
@@ -73,3 +78,53 @@ def test_set_config():
7378
assert r.headers['Authorization'] == sdk_key
7479
finally:
7580
_reset_client()
81+
82+
83+
@pytest.mark.xfail(strict=True, reason="set_config() holds the global write lock across the old client's close()")
84+
def test_set_config_does_not_block_concurrent_get():
85+
"""
86+
INVARIANT: reconfiguring the shared client does not stall unrelated threads that are only
87+
reading it. ldclient.get() is on the hot path of every flag evaluation in an application.
88+
89+
set_config() holds the global write lock (ldclient/__init__.py) across both the construction
90+
of the replacement client and old_client.close(). ReadWriteLock.lock() holds the underlying
91+
mutex for its whole duration, so every concurrent ldclient.get() - which takes a read lock -
92+
blocks until both of those finish.
93+
94+
That couples a network-dependent shutdown to a lock every evaluating thread needs. A close()
95+
that stalls stalls the entire application, not just the thread that called set_config().
96+
"""
97+
_reset_client()
98+
close_duration = 3.0
99+
real_close = LDClient.close
100+
101+
def slow_close(self):
102+
time.sleep(close_duration)
103+
real_close(self)
104+
105+
try:
106+
ldclient.set_config(Config(sdk_key, offline=True))
107+
ldclient.get()
108+
109+
LDClient.close = slow_close # type: ignore[method-assign]
110+
111+
reconfiguring = threading.Event()
112+
113+
def reconfigure():
114+
reconfiguring.set()
115+
ldclient.set_config(Config(sdk_key, offline=True))
116+
117+
threading.Thread(target=reconfigure, name='reconfigure', daemon=True).start()
118+
assert reconfiguring.wait(5)
119+
time.sleep(0.2) # let set_config get inside the write lock
120+
121+
started = time.time()
122+
ldclient.get()
123+
blocked_for = time.time() - started
124+
125+
assert blocked_for < close_duration / 2, (
126+
"ldclient.get() blocked for %.1fs while set_config() was closing the previous client" % blocked_for
127+
)
128+
finally:
129+
LDClient.close = real_close # type: ignore[method-assign]
130+
_reset_client()

0 commit comments

Comments
 (0)