From e7047ec2fcc20e150c0c8cf29addf582638d7e80 Mon Sep 17 00:00:00 2001 From: Yossi Gottlieb Date: Tue, 22 Dec 2020 12:24:20 +0200 Subject: [PATCH] Fix crashes with io-threads-do-reads enabled. (#8230) Normally IO threads should simply read data from the socket into the buffer and attempt to parse it. If a protocol error is detected, a reply is generated which may result with installing a write handler which is not thread safe. This fix delays that until the client is processed back in the main thread. Fixes #8220 --- src/networking.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/src/networking.c b/src/networking.c index b81060cbb..45c504e46 100644 --- a/src/networking.c +++ b/src/networking.c @@ -259,8 +259,14 @@ int prepareClientToWrite(client *c) { if (!c->conn) return C_ERR; /* Fake client for AOF loading. */ /* Schedule the client to write the output buffers to the socket, unless - * it should already be setup to do so (it has already pending data). */ - if (!clientHasPendingReplies(c)) clientInstallWriteHandler(c); + * it should already be setup to do so (it has already pending data). + * + * If CLIENT_PENDING_READ is set, we're in an IO thread and should + * not install a write handler. Instead, it will be done by + * handleClientsWithPendingReadsUsingThreads() upon return. + */ + if (!clientHasPendingReplies(c) && !(c->flags & CLIENT_PENDING_READ)) + clientInstallWriteHandler(c); /* Authorize the caller to queue in the output buffer of this client. */ return C_OK; @@ -3509,6 +3515,12 @@ int handleClientsWithPendingReadsUsingThreads(void) { } } processInputBuffer(c); + + /* We may have pending replies if a thread readQueryFromClient() produced + * replies and did not install a write handler (it can't). + */ + if (!(c->flags & CLIENT_PENDING_WRITE) && clientHasPendingReplies(c)) + clientInstallWriteHandler(c); } /* Update processed count on server */