From ce3e2722eedbc552712def2a7429f10dd03c17f4 Mon Sep 17 00:00:00 2001 From: Aviad Rozenhek Date: Sat, 15 Aug 2026 22:09:27 +0300 Subject: [PATCH] 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) --- ldclient/testing/test_ldclient_singleton.py | 55 +++++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/ldclient/testing/test_ldclient_singleton.py b/ldclient/testing/test_ldclient_singleton.py index f5b474f7..f48bebde 100644 --- a/ldclient/testing/test_ldclient_singleton.py +++ b/ldclient/testing/test_ldclient_singleton.py @@ -1,7 +1,12 @@ import json +import threading +import time + +import pytest import ldclient from ldclient import _reset_client +from ldclient.client import LDClient from ldclient.config import Config from ldclient.testing.http_util import BasicResponse, start_server from ldclient.testing.stub_util import make_put_event, stream_content @@ -73,3 +78,53 @@ def test_set_config(): assert r.headers['Authorization'] == sdk_key finally: _reset_client() + + +@pytest.mark.xfail(strict=True, reason="set_config() holds the global write lock across the old client's close()") +def test_set_config_does_not_block_concurrent_get(): + """ + INVARIANT: reconfiguring the shared client does not stall unrelated threads that are only + reading it. ldclient.get() is on the hot path of every flag evaluation in an application. + + set_config() holds the global write lock (ldclient/__init__.py) across both the construction + of the replacement client and old_client.close(). ReadWriteLock.lock() holds the underlying + mutex for its whole duration, so every concurrent ldclient.get() - which takes a read lock - + blocks until both of those finish. + + That couples a network-dependent shutdown to a lock every evaluating thread needs. A close() + that stalls stalls the entire application, not just the thread that called set_config(). + """ + _reset_client() + close_duration = 3.0 + real_close = LDClient.close + + def slow_close(self): + time.sleep(close_duration) + real_close(self) + + try: + ldclient.set_config(Config(sdk_key, offline=True)) + ldclient.get() + + LDClient.close = slow_close # type: ignore[method-assign] + + reconfiguring = threading.Event() + + def reconfigure(): + reconfiguring.set() + ldclient.set_config(Config(sdk_key, offline=True)) + + threading.Thread(target=reconfigure, name='reconfigure', daemon=True).start() + assert reconfiguring.wait(5) + time.sleep(0.2) # let set_config get inside the write lock + + started = time.time() + ldclient.get() + blocked_for = time.time() - started + + assert blocked_for < close_duration / 2, ( + "ldclient.get() blocked for %.1fs while set_config() was closing the previous client" % blocked_for + ) + finally: + LDClient.close = real_close # type: ignore[method-assign] + _reset_client()