Skip to content

Commit 4dbf7a9

Browse files
fix(cluster): defensive clamp on m_reqs_generated -= at three staging sites (#437)
PR #399 introduced a >= clamp at hold_pipeline; the three sibling sites (connect_shard_connection, handle_cluster_slots, handle_moved) subtract empty_staged.size() with no such guard. Today the invariant -- entries in empty_staged are pre-decrement -- holds, but a refactor could break it silently. Matching the pattern makes the invariant explicit and future-proofs against subtle underflow to 2^64. Refs: 2.4 review #12 (Review 1 + Review 16). Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 20688d3 commit 4dbf7a9

1 file changed

Lines changed: 23 additions & 3 deletions

File tree

cluster_client.cpp

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -241,7 +241,13 @@ bool cluster_client::connect_shard_connection(shard_connection *sc, char *addres
241241
// Commands in the cleared staged queue were already counted in m_reqs_generated at
242242
// staging time. Compensate so a --requests run is not left waiting for responses
243243
// that will never arrive.
244-
m_reqs_generated -= empty_staged.size();
244+
// Defensive clamp: today entries in empty_staged are popped before m_reqs_generated
245+
// is decremented so underflow cannot occur, but guard explicitly so a future
246+
// refactor does not silently wrap to 2^64. Matches the pattern at hold_pipeline.
247+
{
248+
const size_t n = empty_staged.size();
249+
m_reqs_generated -= (m_reqs_generated >= n) ? n : m_reqs_generated;
250+
}
245251
}
246252

247253
// save address and port
@@ -501,7 +507,14 @@ void cluster_client::handle_cluster_slots(protocol_response *r)
501507
if (!m_staged_monitor_commands[i].empty()) {
502508
std::queue<staged_monitor_cmd> empty_staged;
503509
std::swap(m_staged_monitor_commands[i], empty_staged);
504-
m_reqs_generated -= empty_staged.size();
510+
// Defensive clamp: entries in empty_staged are popped before
511+
// m_reqs_generated is decremented so underflow cannot occur today,
512+
// but guard explicitly so a future refactor does not silently wrap
513+
// to 2^64. Matches the pattern at hold_pipeline.
514+
{
515+
const size_t n = empty_staged.size();
516+
m_reqs_generated -= (m_reqs_generated >= n) ? n : m_reqs_generated;
517+
}
505518
}
506519
if (m_connections[i]->get_connection_state() != conn_disconnected) {
507520
m_connections[i]->disconnect();
@@ -1067,7 +1080,14 @@ void cluster_client::handle_moved(unsigned int conn_id, struct timeval timestamp
10671080
std::swap(m_staged_monitor_commands[conn_id], empty_staged);
10681081
// Staged commands were already counted in m_reqs_generated at staging time.
10691082
// Compensate so a --requests run does not hang waiting for phantom responses.
1070-
m_reqs_generated -= empty_staged.size();
1083+
// Defensive clamp: entries in empty_staged are popped before m_reqs_generated
1084+
// is decremented so underflow cannot occur today, but guard explicitly so a
1085+
// future refactor does not silently wrap to 2^64. Matches the pattern at
1086+
// hold_pipeline.
1087+
{
1088+
const size_t n = empty_staged.size();
1089+
m_reqs_generated -= (m_reqs_generated >= n) ? n : m_reqs_generated;
1090+
}
10711091
}
10721092

10731093
// set connection to send 'CLUSTER SLOTS' command

0 commit comments

Comments
 (0)