Skip to content

Commit d38811e

Browse files
remove client id method
Signed-off-by: Ada-Church-Closure <[email protected]>
1 parent 0dabb3d commit d38811e

4 files changed

Lines changed: 29 additions & 33 deletions

File tree

src/connection.h

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -511,13 +511,10 @@ static inline int connIsTLS(connection *conn) {
511511
return conn && conn->type == connectionTypeTls();
512512
}
513513

514-
/* Return 1 on state updated, 0 if otherwise. */
515-
static inline int connUpdateState(connection *conn) {
514+
static inline void connUpdateState(connection *conn) {
516515
if (conn->type->update_state) {
517516
conn->type->update_state(conn);
518-
return 1;
519517
}
520-
return 0;
521518
}
522519

523520
static inline void connSetPostponeUpdateState(connection *conn, int on) {

src/memory_prefetch.c

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,10 @@
99
*/
1010

1111
#include "memory_prefetch.h"
12+
#include "adlist.h"
1213
#include "server.h"
1314
#include "io_threads.h"
15+
#include <stdlib.h>
1416

1517
typedef enum {
1618
PREFETCH_ENTRY, /* Initial state, prefetch entries associated with the given key's hash */
@@ -214,7 +216,7 @@ static void prefetchCommands(void) {
214216
}
215217

216218
/* Processes all the prefetched commands in the current batch. */
217-
void processClientsCommandsBatch(void) {
219+
void processClientsCommandsBatch(list *handled_clients) {
218220
if (!batch || batch->client_count == 0) return;
219221

220222
/* If executed_commands is not 0,
@@ -231,7 +233,10 @@ void processClientsCommandsBatch(void) {
231233
/* Set the client to null immediately to avoid accessing it again recursively when ProcessingEventsWhileBlocked */
232234
batch->clients[i] = NULL;
233235
batch->executed_commands++;
234-
if (processPendingCommandAndInputBuffer(c) != C_ERR) beforeNextClient(c);
236+
if (processPendingCommandAndInputBuffer(c) != C_ERR) {
237+
beforeNextClient(c);
238+
if (handled_clients) listAddNodeTail(handled_clients, c);
239+
}
235240
}
236241

237242
resetCommandsBatch();
@@ -260,7 +265,7 @@ static void addCommandToBatch(struct serverCommand *cmd, robj **argv, int argc,
260265
* if it becomes full.
261266
*
262267
* Returns C_OK if the command was added successfully, C_ERR otherwise. */
263-
int addCommandToBatchAndProcessIfFull(client *c) {
268+
int addCommandToBatchAndProcessIfFull(client *c, list *handled_clients) {
264269
if (!batch) return C_ERR;
265270

266271
batch->clients[batch->client_count++] = c;
@@ -283,7 +288,7 @@ int addCommandToBatchAndProcessIfFull(client *c) {
283288
* We also check the client count to handle cases where
284289
* no keys exist for the clients' commands. */
285290
if (batch->client_count == batch->max_prefetch_size || batch->key_count == batch->max_prefetch_size) {
286-
processClientsCommandsBatch();
291+
processClientsCommandsBatch(handled_clients);
287292
}
288293

289294
return C_OK;

src/memory_prefetch.h

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
11
#ifndef MEMORY_PREFETCH_H
22
#define MEMORY_PREFETCH_H
33

4+
#include "adlist.h"
5+
46
struct client;
57

68
void prefetchCommandsBatchInit(void);
7-
void processClientsCommandsBatch(void);
8-
int addCommandToBatchAndProcessIfFull(struct client *c);
9+
void processClientsCommandsBatch(list *handled_clients);
10+
int addCommandToBatchAndProcessIfFull(struct client *c, list *handled_clients);
911
void removeClientFromPendingCommandsBatch(struct client *c);
1012
int onMaxBatchSizeChange(const char **err);
1113

src/networking.c

Lines changed: 15 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@
2727
* POSSIBILITY OF SUCH DAMAGE.
2828
*/
2929

30+
#include "adlist.h"
31+
#include "list.h"
3032
#include "server.h"
3133
#include "cluster.h"
3234
#include "cluster_slot_stats.h"
@@ -6373,16 +6375,15 @@ int processIOThreadsReadDone(void) {
63736375
if (ProcessingEventsWhileBlocked) {
63746376
/* When ProcessingEventsWhileBlocked we may call processIOThreadsReadDone recursively.
63756377
* In this case, there may be some clients left in the batch waiting to be processed. */
6376-
processClientsCommandsBatch();
6378+
processClientsCommandsBatch(NULL);
63776379
}
63786380

63796381
if (listLength(server.clients_pending_io_read) == 0) return 0;
63806382
int processed = 0;
63816383
/* Collect clients handled in this iteration to continue parsing any
63826384
* residual buffered data synchronously after batch execution (important
6383-
* for edge-triggered transports like RDMA). Use IDs instead of raw client
6384-
* pointers to avoid UAF if a client is freed while executing the batch. */
6385-
list *handled_clients_ids = listCreate();
6385+
* for edge-triggered transports like RDMA). */
6386+
list *handled_clients = listCreate();
63866387
listNode *ln;
63876388

63886389
listNode *next = listFirst(server.clients_pending_io_read);
@@ -6444,39 +6445,30 @@ int processIOThreadsReadDone(void) {
64446445

64456446
size_t list_length_before_command_execute = listLength(server.clients_pending_io_read);
64466447
/* try to add the command to the batch */
6447-
int ret = addCommandToBatchAndProcessIfFull(c);
6448+
int ret = addCommandToBatchAndProcessIfFull(c, handled_clients);
64486449
/* If the command was not added to the commands batch, process it immediately */
64496450
if (ret == C_ERR) {
6450-
if (processPendingCommandAndInputBuffer(c) == C_OK) beforeNextClient(c);
6451+
if (processPendingCommandAndInputBuffer(c) == C_OK) {
6452+
beforeNextClient(c);
6453+
listAddNodeTail(handled_clients,c);
6454+
}
64516455
}
64526456
if (list_length_before_command_execute != listLength(server.clients_pending_io_read)) {
64536457
/* A client was unlink from the list possibly making the next node invalid */
64546458
next = listFirst(server.clients_pending_io_read);
64556459
}
6456-
6457-
/* Track the client by ID so we can drain any leftover buffered data even if no
6458-
* further network events arrive. */
6459-
listAddNodeTail(handled_clients_ids, (void *)(uintptr_t)c->id);
64606460
}
64616461

6462-
processClientsCommandsBatch();
6462+
processClientsCommandsBatch(handled_clients);
64636463

6464-
/* For clients handled in this iteration, if residual data remains in
6465-
* querybuf, handle it now to avoid edge-triggered lost wakeups. Clients may
6466-
* have been freed while executing the batch, so re-lookup by ID. */
64676464
listIter handled_li;
64686465
listNode *handled_ln;
6469-
listRewind(handled_clients_ids, &handled_li);
6466+
listRewind(handled_clients, &handled_li);
64706467
while ((handled_ln = listNext(&handled_li))) {
6471-
uint64_t id = (uint64_t)(uintptr_t)listNodeValue(handled_ln);
6472-
client *c = lookupClientByID(id);
6473-
if (!c) continue;
6474-
6475-
if (connUpdateState(c->conn)) {
6476-
processPendingCommandAndInputBuffer(c); /* try to handle new arrival data if possible */
6477-
}
6468+
client *c = listNodeValue(handled_ln);
6469+
connUpdateState(c->conn);
64786470
}
6479-
listRelease(handled_clients_ids);
6471+
listRelease(handled_clients);
64806472

64816473
return processed;
64826474
}

0 commit comments

Comments
 (0)