Skip to content

Commit 553fbf1

Browse files
committed
fix(tokens): claim redis socket tokens atomically
Signed-off-by: Anas Khan <83116240+anxkhn@users.noreply.github.com>
1 parent 65a2889 commit 553fbf1

3 files changed

Lines changed: 67 additions & 36 deletions

File tree

news/6740.bugfix.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Redis-backed socket token claims are now atomic across workers, preventing simultaneous connections from sharing client state.

reflex/utils/token_manager.py

Lines changed: 14 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -309,36 +309,27 @@ async def link_token_to_sid(self, token: str, sid: str) -> str | None:
309309
# Make sure the update subscriber is running
310310
self._ensure_socket_record_task()
311311

312-
# Check Redis for cross-worker duplicates
313-
redis_key = self._get_redis_key(token)
312+
socket_record = SocketRecord(instance_id=self.instance_id, sid=sid)
313+
socket_record_data = pickle.dumps(socket_record)
314+
new_token = None
314315

315316
try:
316-
token_exists_in_redis = await self.redis.exists(redis_key)
317+
while not await self.redis.set(
318+
self._get_redis_key(token),
319+
socket_record_data,
320+
ex=self.token_expiration,
321+
nx=True,
322+
):
323+
token = new_token = _get_new_token()
317324
except Exception as e:
318-
console.error(f"Redis error checking token existence: {e}")
319-
return await super().link_token_to_sid(token, sid)
320-
321-
new_token = None
322-
if token_exists_in_redis:
323-
# Duplicate exists somewhere - generate new token
324-
token = new_token = _get_new_token()
325-
redis_key = self._get_redis_key(new_token)
325+
console.error(f"Redis error claiming token: {e}")
326+
fallback_token = await super().link_token_to_sid(token, sid)
327+
return fallback_token or new_token
326328

327329
# Store in local dicts
328-
socket_record = self.token_to_socket[token] = SocketRecord(
329-
instance_id=self.instance_id, sid=sid
330-
)
330+
self.token_to_socket[token] = socket_record
331331
self.sid_to_token[sid] = token
332332

333-
# Store in Redis if possible
334-
try:
335-
await self.redis.set(
336-
redis_key,
337-
pickle.dumps(socket_record),
338-
ex=self.token_expiration,
339-
)
340-
except Exception as e:
341-
console.error(f"Redis error storing token: {e}")
342333
# Return the new token if one was generated
343334
return new_token
344335

tests/units/utils/test_token_manager.py

Lines changed: 52 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -291,18 +291,16 @@ async def test_link_token_to_sid_normal_case(self, manager, mock_redis):
291291
mock_redis: Mock Redis client fixture.
292292
"""
293293
token, sid = "token1", "sid1"
294-
mock_redis.exists.return_value = False
294+
mock_redis.set.return_value = True
295295

296296
result = await manager.link_token_to_sid(token, sid)
297297

298298
assert result is None
299-
mock_redis.exists.assert_called_once_with(
300-
f"token_manager_socket_record_{token}"
301-
)
302299
mock_redis.set.assert_called_once_with(
303300
f"token_manager_socket_record_{token}",
304301
pickle.dumps(SocketRecord(instance_id=manager.instance_id, sid=sid)),
305302
ex=3600,
303+
nx=True,
306304
)
307305
assert manager.token_to_socket[token].sid == sid
308306
assert manager.sid_to_token[sid] == token
@@ -324,7 +322,6 @@ async def test_link_token_to_sid_reconnection_skips_redis(
324322
result = await manager.link_token_to_sid(token, sid)
325323

326324
assert result is None
327-
mock_redis.exists.assert_not_called()
328325
mock_redis.set.assert_not_called()
329326

330327
async def test_link_token_to_sid_duplicate_detected(self, manager, mock_redis):
@@ -335,21 +332,20 @@ async def test_link_token_to_sid_duplicate_detected(self, manager, mock_redis):
335332
mock_redis: Mock Redis client fixture.
336333
"""
337334
token, sid = "token1", "sid1"
338-
mock_redis.exists.return_value = True
335+
mock_redis.set.side_effect = [False, True]
339336

340337
result = await manager.link_token_to_sid(token, sid)
341338

342339
assert result is not None
343340
assert result != token
344341
assert len(result) == 36 # UUID4 length
345342

346-
mock_redis.exists.assert_called_once_with(
347-
f"token_manager_socket_record_{token}"
348-
)
349-
mock_redis.set.assert_called_once_with(
343+
assert mock_redis.set.await_count == 2
344+
mock_redis.set.assert_awaited_with(
350345
f"token_manager_socket_record_{result}",
351346
pickle.dumps(SocketRecord(instance_id=manager.instance_id, sid=sid)),
352347
ex=3600,
348+
nx=True,
353349
)
354350
assert manager.token_to_sid[result] == sid
355351
assert manager.sid_to_token[sid] == result
@@ -362,7 +358,7 @@ async def test_link_token_to_sid_redis_error_fallback(self, manager, mock_redis)
362358
mock_redis: Mock Redis client fixture.
363359
"""
364360
token, sid = "token1", "sid1"
365-
mock_redis.exists.side_effect = Exception("Redis connection error")
361+
mock_redis.set.side_effect = Exception("Redis connection error")
366362

367363
with patch.object(
368364
LocalTokenManager, "link_token_to_sid", new_callable=AsyncMock
@@ -384,7 +380,6 @@ async def test_link_token_to_sid_redis_set_error_continues(
384380
mock_redis: Mock Redis client fixture.
385381
"""
386382
token, sid = "token1", "sid1"
387-
mock_redis.exists.return_value = False
388383
mock_redis.set.side_effect = Exception("Redis set error")
389384

390385
result = await manager.link_token_to_sid(token, sid)
@@ -393,6 +388,50 @@ async def test_link_token_to_sid_redis_set_error_continues(
393388
assert manager.token_to_sid[token] == sid
394389
assert manager.sid_to_token[sid] == token
395390

391+
async def test_link_token_to_sid_claims_token_atomically(self, mock_redis):
392+
"""Test concurrent managers cannot both claim the same token.
393+
394+
Args:
395+
mock_redis: Mock Redis client fixture.
396+
"""
397+
redis_values = {}
398+
exists_calls = 0
399+
both_checked = asyncio.Event()
400+
401+
async def exists(key):
402+
nonlocal exists_calls
403+
result = key in redis_values
404+
exists_calls += 1
405+
if exists_calls == 2:
406+
both_checked.set()
407+
await both_checked.wait()
408+
return result
409+
410+
def set_value(key, value, *, ex, nx=False):
411+
if nx and key in redis_values:
412+
return False
413+
redis_values[key] = value
414+
return True
415+
416+
mock_redis.exists.side_effect = exists
417+
mock_redis.set.side_effect = set_value
418+
with patch("reflex_base.config.get_config") as mock_get_config:
419+
mock_get_config.return_value.redis_token_expiration = 3600
420+
managers = [RedisTokenManager(mock_redis), RedisTokenManager(mock_redis)]
421+
422+
with patch.object(RedisTokenManager, "_ensure_socket_record_task"):
423+
results = await asyncio.gather(
424+
managers[0].link_token_to_sid("token1", "sid1"),
425+
managers[1].link_token_to_sid("token1", "sid2"),
426+
)
427+
428+
assert sum(result is None for result in results) == 1
429+
claimed_tokens = {
430+
manager.sid_to_token[sid]
431+
for manager, sid in zip(managers, ("sid1", "sid2"), strict=True)
432+
}
433+
assert len(claimed_tokens) == 2
434+
396435
async def test_disconnect_token_owned_locally(self, manager, mock_redis):
397436
"""Test disconnect cleans up both Redis and local mappings when owned locally.
398437
@@ -465,7 +504,7 @@ async def test_various_redis_errors_handled_gracefully(
465504
redis_error: Exception to test error handling.
466505
"""
467506
token, sid = "token1", "sid1"
468-
mock_redis.exists.side_effect = redis_error
507+
mock_redis.set.side_effect = redis_error
469508

470509
with patch.object(
471510
LocalTokenManager, "link_token_to_sid", new_callable=AsyncMock

0 commit comments

Comments
 (0)