diff --git a/src/networking.c b/src/networking.c index d02d02b5..6dc4864b 100644 --- a/src/networking.c +++ b/src/networking.c @@ -685,12 +685,50 @@ void disconnectSlaves(void) { } } -void freeClient(client *c) { +/* Remove the specified client from global lists where the client could + * be referenced, not including the Pub/Sub channels. + * This is used by freeClient() and replicationCacheMaster(). */ +void unlinkClient(client *c) { listNode *ln; - /* If this is marked as current client unset it */ + /* If this is marked as current client unset it. */ if (server.current_client == c) server.current_client = NULL; + /* Certain operations must be done only if the client has an active socket. + * If the client was already unlinked or if it's a "fake client" the + * fd is already set to -1. */ + if (c->fd != -1) { + /* Remove from the list of active clients. */ + ln = listSearchKey(server.clients,c); + serverAssert(ln != NULL); + listDelNode(server.clients,ln); + + /* Unregister async I/O handlers and close the socket. */ + aeDeleteFileEvent(server.el,c->fd,AE_READABLE); + aeDeleteFileEvent(server.el,c->fd,AE_WRITABLE); + close(c->fd); + c->fd = -1; + } + + /* Remove from the list of pending writes if needed. */ + if (c->flags & CLIENT_PENDING_WRITE) { + ln = listSearchKey(server.clients_pending_write,c); + serverAssert(ln != NULL); + listDelNode(server.clients_pending_write,ln); + } + + /* When client was just unblocked because of a blocking operation, + * remove it from the list of unblocked clients. */ + if (c->flags & CLIENT_UNBLOCKED) { + ln = listSearchKey(server.unblocked_clients,c); + serverAssert(ln != NULL); + listDelNode(server.unblocked_clients,ln); + } +} + +void freeClient(client *c) { + listNode *ln; + /* If it is our master that's beging disconnected we should make sure * to cache the state to try a partial resynchronization later. * @@ -732,36 +770,14 @@ void freeClient(client *c) { dictRelease(c->pubsub_channels); listRelease(c->pubsub_patterns); - /* Close socket, unregister events, and remove list of replies and - * accumulated arguments. */ - if (c->fd != -1) { - aeDeleteFileEvent(server.el,c->fd,AE_READABLE); - aeDeleteFileEvent(server.el,c->fd,AE_WRITABLE); - close(c->fd); - } + /* Free data structures. */ listRelease(c->reply); freeClientArgv(c); - /* Remove from the list of clients */ - if (c->fd != -1) { - ln = listSearchKey(server.clients,c); - serverAssert(ln != NULL); - listDelNode(server.clients,ln); - } - - /* Remove from the list of pending writes if needed. */ - if (c->flags & CLIENT_PENDING_WRITE) { - ln = listSearchKey(server.clients_pending_write,c); - listDelNode(server.clients_pending_write,ln); - } - - /* When client was just unblocked because of a blocking operation, - * remove it from the list of unblocked clients. */ - if (c->flags & CLIENT_UNBLOCKED) { - ln = listSearchKey(server.unblocked_clients,c); - serverAssert(ln != NULL); - listDelNode(server.unblocked_clients,ln); - } + /* Unlink the client: this will close the socket, remove the I/O + * handlers, and remove references of the client from different + * places where active clients may be referenced. */ + unlinkClient(c); /* Master/slave cleanup Case 1: * we lost the connection with a slave. */ diff --git a/src/replication.c b/src/replication.c index 6b5f44dd..7ab64636 100644 --- a/src/replication.c +++ b/src/replication.c @@ -1846,36 +1846,16 @@ void replicationSendAck(void) { * handshake in order to reactivate the cached master. */ void replicationCacheMaster(client *c) { - listNode *ln; - serverAssert(server.master != NULL && server.cached_master == NULL); serverLog(LL_NOTICE,"Caching the disconnected master state."); - /* Remove from the list of clients, we don't want this client to be - * listed by CLIENT LIST or processed in any way by batch operations. */ - ln = listSearchKey(server.clients,c); - serverAssert(ln != NULL); - listDelNode(server.clients,ln); - - /* Remove from the list of clients with pending writes as well. */ - if (c->flags & CLIENT_PENDING_WRITE) { - ln = listSearchKey(server.clients_pending_write,c); - if (ln) listDelNode(server.clients_pending_write,ln); - } + /* Unlink the client from the server structures. */ + unlinkClient(c); /* Save the master. Server.master will be set to null later by * replicationHandleMasterDisconnection(). */ server.cached_master = server.master; - /* Remove the event handlers and close the socket. We'll later reuse - * the socket of the new connection with the master during PSYNC. */ - aeDeleteFileEvent(server.el,c->fd,AE_READABLE); - aeDeleteFileEvent(server.el,c->fd,AE_WRITABLE); - close(c->fd); - - /* Set fd to -1 so that we can safely call freeClient(c) later. */ - c->fd = -1; - /* Invalidate the Peer ID cache. */ if (c->peerid) { sdsfree(c->peerid); diff --git a/src/server.h b/src/server.h index 400f1ff8..b5a24829 100644 --- a/src/server.h +++ b/src/server.h @@ -1112,6 +1112,7 @@ int clientsArePaused(void); int processEventsWhileBlocked(void); void handleClientsWithPendingWrites(void); int clientHasPendingReplies(client *c); +void unlinkClient(client *c); #ifdef __GNUC__ void addReplyErrorFormat(client *c, const char *fmt, ...)